# v1.6 Track A — P&ID Expert and Reader Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Make the server a P&ID expert (code-authored ISO 10628-2 / ISA-5.1 symbol catalogue as real blocks with tags and ports, orthogonal line drawing with ISA-5.1 line classes, an ISA-5.1 tag grammar) and a P&ID reader (`pid_graph` turns any P&ID into a connectivity graph; instrument index, line list and equipment list derive from it; six critique focuses join `drawing_finalize`). **Architecture:** Two new generic backend contracts (`block_define`, `entity_get/set_xdata`) implemented on both engines; everything else composes existing primitives from a new `engineering/pid/` package (symbols as data, pure routing, pure tag grammar, a graph builder that reads the drawing back). Nine `pid_*` tools in a new `SECTION 17` of `server.py`, gated by a new `TOOL_PACKS` mechanism. **Tech Stack:** Python 3.11+, FastMCP 3, ezdxf 1.4, pywin32 (COM, fake-tested + live smoke), pytest-asyncio. **Spec:** `docs/superpowers/specs/2026-09-15-v1.6-pid-design.md` (parent: `docs/superpowers/specs/2026-09-15-v1.6-roadmap-design.md`, track A). ## Global Constraints - Run every command through the project environment: `uv run --frozen pytest ...`, `uv run --frozen python ...` (the shell's default `python` is 3.14 without the project's dependencies). - Every coordinate in or out of a tool is WCS on both engines; symbol geometry is authored in symbol-local mm and transformed at insertion. - Every new tool needs: a `@cad_tool(summary=..., cost=...)` line above its `@mcp.tool(...)`, `tags={...}`, an alias record in `discovery/aliases.py` (the gate `tests/test_discovery_aliases.py::test_every_registered_tool_has_an_alias_record` fails otherwise), a regenerated `docs/tool-inventory.json` (`uv run --frozen python scripts/generate_tool_inventory.py`), the README release-snapshot line updated to the new tool count, and the `SECTION` header comment count updated (`tests/test_release_consistency.py::test_section_header_counts_match_decorators` parses decorators per section). - The README release-snapshot line today reads `> **v1.5 release snapshot:** 154 tools · 6 resources · 5 prompt templates · 1369 collected tests.` — bump the tool/resource counts in the same task that adds them; leave the minor version at `v1.5` until the release task (a `v1.6` label there requires `pyproject.toml` to say 1.6). - Both engines implement every new contract method as `@abstractmethod` (no `@capability` in this track). COM paths are unit-tested against fake ActiveX objects (the `tests/test_block_from_entities.py` pattern) and executed once live by `scripts/smoke_pid_com.py` in the last task. - Validation happens **before** any write: a malformed request raises `TypeError`/`ValueError` naming the offending index/key, and nothing is created. - Authored standards data (symbols, ISA letter tables) is pinned by tests a reader can check against the standard. - P&ID critique focuses return `[]` on a drawing with no P&ID nodes. - Reference sizes (symbol-local mm): instrument bubble Ø10; valve body 8 × 4; pump Ø8; grid 5; equipment `TAG` text height 2.5; bubble `FUNC`/`LOOP` height 2.0; stub 5. - Commit after every task with a conventional-commit message; end every commit message with the two attribution lines this session's harness requires (`Co-Authored-By: Claude Opus 5 (1M context) ` and `Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd`). If `git commit` reports "Author identity unknown", pass `-c user.name="Umut CAN" -c user.email="umutcan.edizaslan@gmail.com"` (the repository's existing author) — do not change global git config. ## File Structure | Path | Responsibility | |---|---| | `backends/block_specs.py` (new) | Validation/normalisation of `block_define` primitive and ATTDEF specs; SOLID vertex order. Shared by both engines. | | `backends/xdata_specs.py` (new) | XDATA app-name validation, value ↔ (group code, value) encoding, size limits, per-app splitting. Shared by both engines. | | `backends/contracts/blocks.py` | `+ block_define` | | `backends/contracts/entity_query.py` | `+ entity_get_xdata`, `+ entity_set_xdata` | | `backends/ezdxf_backend.py` | `block_insert` attribute fix; `block_define`; XDATA methods | | `backends/com_backend.py` | `block_define`; XDATA methods | | `engineering/pid/__init__.py` (new) | Public re-exports | | `engineering/pid/geometry.py` (new) | Primitive-spec helpers (line/circle/arc/polyline/text/solid/dashed_line/regular_polygon/bbox_of/attdef) | | `engineering/pid/symbols.py` (new) | `Port`, `SymbolSpec`, registry, `resolve`, `list_symbols`, `all_specs`, `transform_port`, instrument/marker/connector builders | | `engineering/pid/symbols_valves.py` (new) | 11 valve bodies × 6 actuators | | `engineering/pid/symbols_equipment.py` (new) | rotating (6), heat (4), misc (7) | | `engineering/pid/symbols_vessels.py` (new) | 6 parametric vessels | | `engineering/pid/xdata.py` (new) | `ACADMCP_PID` payload encode/decode/read/write, payload builders | | `engineering/pid/tags.py` (new) | ISA-5.1 grammar + tables, equipment prefix map, `parse_tag`, `split_instrument_tag` | | `engineering/pid/insert.py` (new) | `ensure_layer`, `ensure_block`, `insert_symbol` | | `engineering/pid/lines.py` (new) | `LINE_CLASSES`, `route`, `label_placement`, `marker_positions`, `count_crossings`, `format_line_number` (pure) | | `engineering/pid/drawlines.py` (new) | `draw_line(backend, ...)` orchestration | | `engineering/pid/graph.py` (new) | `build_graph(backend, ...)` | | `engineering/pid/deliverables.py` (new) | `instrument_index`, `line_list`, `equipment_list`, `write_csv` | | `engineering/pid/critique.py` (new) | six `pid_*` focus checks over one cached graph | | `engineering/pid/spec.py` (new) | `validate_spec`, `run_spec(backend, spec, dry_run)` | | `engineering/layers.py` | `PID_LAYERS` + `PROCESS-LINE-TEXT` | | `engineering/plan_spec.py` | `CritiqueFocus` + 6 values | | `engineering/critique.py` | shared-context dispatch for the pid focuses | | `engineering/preflight.py` | `LAYER_SET_INTENT_MISMATCH` conflict | | `config.py` | `tool_packs` | | `server.py` | SECTION 5 (+2), SECTION 7 (+1), SECTION 17 (9 tools), `TOOL_PACKS`, 2 resources, prompt rewrite, `drawing_plan` warning | | `discovery/aliases.py` | 12 alias records | | `benchmarks/tasks_v4.py` (new), `benchmarks/adapters/autocad_mcp_pro.py`, `benchmarks/correctness_suite.py`, `benchmarks/run_competitors.py`, `benchmarks/render_matrix_chart.py` | `pid_roundtrip` task, +3 correctness checks | | `scripts/render_pid_catalog.py`, `scripts/render_readme_pid.py`, `scripts/smoke_pid_com.py` (new) | contact sheet, README showcase, live COM smoke | | `tests/test_block_insert_attributes.py`, `tests/test_block_specs.py`, `tests/test_block_define.py`, `tests/test_xdata.py`, `tests/test_pid_xdata.py`, `tests/test_pid_tags.py`, `tests/test_pid_symbols.py`, `tests/test_pid_insert.py`, `tests/test_pid_lines.py`, `tests/test_pid_line_draw.py`, `tests/test_pid_graph.py`, `tests/test_pid_deliverables.py`, `tests/test_pid_critique.py`, `tests/test_pid_from_spec.py`, `tests/test_tool_packs.py`, `tests/test_pid_resources_prompt.py`, `tests/test_benchmark_v4.py`, `tests/data/isa51_tags.json` | tests and fixtures | --- ### Task 1: Fix `block_insert(attributes=…)` on the headless engine The ezdxf `block_insert` uses `msp.add_auto_blockref`, which wraps the INSERT in an anonymous `*U…` block: the returned handle references `*U1`, and `block_get_attributes` on it returns `{}`. Verified 2026-09-15 on v1.5.1. Everything in this track reads attributes back, so this goes first. **Files:** - Modify: `backends/ezdxf_backend.py` (`block_insert`, around line 4436) - Test: `tests/test_block_insert_attributes.py` **Interfaces:** - Produces: `block_insert(name, x, y, scale_x, scale_y, rotation, attributes, layer) -> EntityInfo` whose `properties["block_name"] == name` and whose ATTRIBs are attached to the INSERT itself. - [ ] **Step 1: Write the failing test** ```python # tests/test_block_insert_attributes.py """block_insert(attributes=...) must reference the named block, not an anonymous wrapper. ezdxf's `add_auto_blockref` wraps the INSERT in an anonymous `*U` block. The handle it returns then reports block_name `*U1`, and `block_get_attributes` finds no ATTRIBs on it — an insert with attributes silently lost them. """ from __future__ import annotations import pytest pytestmark = pytest.mark.asyncio def _define_tagged_block(backend, name: str = "TB") -> None: blk = backend._doc.blocks.new(name=name) blk.add_line((0, 0), (10, 0)) blk.add_attdef("TAG", insert=(0, 3), text="", dxfattribs={"height": 2.5}) async def test_insert_with_attributes_references_the_named_block(backend): _define_tagged_block(backend) ref = await backend.block_insert("TB", 10, 10, attributes={"TAG": "P-101"}) info = await backend.entity_get(ref.handle) assert info.properties["block_name"] == "TB" assert await backend.block_get_attributes(ref.handle) == {"TAG": "P-101"} anonymous = [b.name for b in backend._doc.blocks if b.name.startswith("*U")] assert anonymous == [], "no anonymous wrapper block may be created" async def test_insert_ignores_values_for_tags_the_block_does_not_define(backend): _define_tagged_block(backend) ref = await backend.block_insert("TB", 0, 0, attributes={"TAG": "V-1", "NOPE": "x"}) assert await backend.block_get_attributes(ref.handle) == {"TAG": "V-1"} async def test_insert_without_attributes_is_unchanged(backend): _define_tagged_block(backend) ref = await backend.block_insert("TB", 0, 0) info = await backend.entity_get(ref.handle) assert info.properties["block_name"] == "TB" assert await backend.block_get_attributes(ref.handle) == {} ``` - [ ] **Step 2: Run it to verify it fails** Run: `uv run --frozen pytest tests/test_block_insert_attributes.py -v` Expected: the first two tests FAIL (`block_name == '*U1'`, attributes `{}`); the third passes. - [ ] **Step 3: Replace `add_auto_blockref` with `add_blockref(...).add_auto_attribs(...)`** In `backends/ezdxf_backend.py::block_insert`, replace the `if attributes: ... else: ...` pair with: ```python ref = msp.add_blockref(name, (float(x), float(y)), dxfattribs=attribs) if attributes: # `add_auto_blockref` wraps the INSERT in an anonymous *U block, # so the returned handle names `*U1` and carries no ATTRIBs. # `add_auto_attribs` attaches ATTRIBs to this INSERT for every # ATTDEF the definition has; values for tags it lacks are ignored. ref.add_auto_attribs({str(k): str(v) for k, v in attributes.items()}) ``` - [ ] **Step 4: Run the tests to verify they pass** Run: `uv run --frozen pytest tests/test_block_insert_attributes.py tests/test_layer_block.py tests/test_block_from_entities.py -v` Expected: all PASS. - [ ] **Step 5: Commit** ```bash git add backends/ezdxf_backend.py tests/test_block_insert_attributes.py git commit -m "fix(ezdxf): block_insert with attributes referenced an anonymous *U block and lost its ATTRIBs" ``` --- ### Task 2: Shared primitive/ATTDEF spec validation (`backends/block_specs.py`) **Files:** - Create: `backends/block_specs.py` - Test: `tests/test_block_specs.py` **Interfaces:** - Produces: - `ALLOWED_TYPES = ("line", "circle", "arc", "polyline", "text", "solid")`, `TEXT_ALIGNMENTS = ("left", "center", "right", "middle_center")` - `validate_entity_specs(entities: list[dict]) -> list[dict]` — normalised copies (floats; `layer` default `"0"`; polyline `points: list[tuple[float, float]]`, `closed: bool`, `bulges: list[float] | None`; text `rotation_deg`, `align`; solid `points` 3–4 pairs). Raises `ValueError` when empty, `TypeError` naming `entities[i]` and the key otherwise. - `validate_attdef_specs(attdefs: list[dict]) -> list[dict]` — keys `tag, prompt, default, x, y, height, rotation_deg, align, invisible`; tag `^[A-Z][A-Z0-9_]{0,30}$` and unique. - `solid_vertices(points) -> list[tuple[float, float]]` — DXF SOLID bow-tie order (`[a, b, d, c]` for a quad, unchanged for a triangle). - [ ] **Step 1: Write the failing tests** ```python # tests/test_block_specs.py """block_define request validation: refuse the whole request before any write.""" from __future__ import annotations import pytest from backends.block_specs import ( ALLOWED_TYPES, solid_vertices, validate_attdef_specs, validate_entity_specs, ) def test_allowed_types_are_the_documented_six(): assert ALLOWED_TYPES == ("line", "circle", "arc", "polyline", "text", "solid") def test_empty_entities_is_refused(): with pytest.raises(ValueError, match="empty"): validate_entity_specs([]) def test_each_type_normalises_to_floats_and_layer_zero(): out = validate_entity_specs( [ {"type": "line", "x1": 0, "y1": 0, "x2": 1, "y2": 0}, {"type": "circle", "cx": 0, "cy": 0, "r": 2}, {"type": "arc", "cx": 0, "cy": 0, "r": 2, "start_deg": 0, "end_deg": 180}, {"type": "polyline", "points": [[0, 0], [1, 0], [1, 1]], "closed": True}, {"type": "text", "text": "M", "x": 0, "y": 0, "height": 2}, {"type": "solid", "points": [[0, 0], [1, 0], [0, 1]]}, ] ) assert [e["type"] for e in out] == list(ALLOWED_TYPES) assert out[0] == {"type": "line", "layer": "0", "x1": 0.0, "y1": 0.0, "x2": 1.0, "y2": 0.0} assert out[3]["points"] == [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0)] assert out[3]["closed"] is True and out[3]["bulges"] is None assert out[4]["align"] == "left" and out[4]["rotation_deg"] == 0.0 assert out[5]["points"] == [(0.0, 0.0), (1.0, 0.0), (0.0, 1.0)] @pytest.mark.parametrize( "bad, fragment", [ ({"type": "square"}, "type"), ({"type": "line", "x1": 0, "y1": 0, "x2": 1}, "y2"), ({"type": "circle", "cx": 0, "cy": 0, "r": 0}, "r"), ({"type": "circle", "cx": 0, "cy": 0, "r": float("nan")}, "finite"), ({"type": "text", "text": "M", "x": 0, "y": 0, "height": -1}, "height"), ({"type": "text", "text": 5, "x": 0, "y": 0, "height": 1}, "text"), ({"type": "text", "text": "M", "x": 0, "y": 0, "height": 1, "align": "top"}, "align"), ({"type": "polyline", "points": [[0, 0]]}, "points"), ({"type": "polyline", "points": [[0, 0], [1]]}, "points[1]"), ({"type": "polyline", "points": [[0, 0], [1, 1]], "bulges": [0.5]}, "bulges"), ({"type": "polyline", "points": [[0, 0], [1, 1]], "closed": "yes"}, "closed"), ({"type": "solid", "points": [[0, 0], [1, 0]]}, "points"), ({"type": "solid", "points": [[0, 0], [1, 0], [1, 1], [0, 1], [2, 2]]}, "at most 4"), ({"type": "line", "x1": 0, "y1": 0, "x2": 1, "y2": 0, "layer": ""}, "layer"), ({"type": "line", "x1": True, "y1": 0, "x2": 1, "y2": 0}, "x1"), ], ) def test_bad_entity_names_the_index_and_key(bad, fragment): good = {"type": "line", "x1": 0, "y1": 0, "x2": 1, "y2": 0} with pytest.raises(TypeError, match=r"entities\[1\]") as exc: validate_entity_specs([good, bad]) assert fragment in str(exc.value) def test_attdefs_normalise_and_default(): out = validate_attdef_specs([{"tag": "TAG", "x": 0, "y": 3, "height": 2.5}]) assert out == [ { "tag": "TAG", "prompt": "TAG", "default": "", "x": 0.0, "y": 3.0, "height": 2.5, "rotation_deg": 0.0, "align": "left", "invisible": False, } ] @pytest.mark.parametrize( "bad, fragment", [ ({"tag": "tag", "x": 0, "y": 0, "height": 1}, "tag"), ({"tag": "T-1", "x": 0, "y": 0, "height": 1}, "tag"), ({"tag": "TAG", "x": 0, "y": 0, "height": 0}, "height"), ({"tag": "TAG", "x": 0, "y": 0, "height": 1, "invisible": 1}, "invisible"), ({"tag": "TAG", "x": 0, "y": 0, "height": 1, "default": 3}, "default"), ], ) def test_bad_attdef_names_the_index_and_key(bad, fragment): with pytest.raises(TypeError, match=r"attdefs\[0\]") as exc: validate_attdef_specs([bad]) assert fragment in str(exc.value) def test_duplicate_attdef_tags_are_refused(): with pytest.raises(TypeError, match="duplicate"): validate_attdef_specs( [ {"tag": "TAG", "x": 0, "y": 0, "height": 1}, {"tag": "TAG", "x": 0, "y": 5, "height": 1}, ] ) def test_solid_vertices_use_dxf_bowtie_order(): quad = [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 1.0)] assert solid_vertices(quad) == [(0.0, 0.0), (1.0, 0.0), (0.0, 1.0), (1.0, 1.0)] tri = [(0.0, 0.0), (1.0, 0.0), (0.0, 1.0)] assert solid_vertices(tri) == tri ``` - [ ] **Step 2: Run them to verify they fail** Run: `uv run --frozen pytest tests/test_block_specs.py -v` Expected: FAIL with `ModuleNotFoundError: No module named 'backends.block_specs'`. - [ ] **Step 3: Write the module** ```python # backends/block_specs.py """Typed primitive specs for ``block_define``, validated before any write. Both engines consume the same normalised specs, so a malformed request is refused identically on COM and ezdxf and nothing is written on either. Every error names the offending entry (``entities[i]`` / ``attdefs[i]``) and key. """ from __future__ import annotations import math import re ALLOWED_TYPES = ("line", "circle", "arc", "polyline", "text", "solid") TEXT_ALIGNMENTS = ("left", "center", "right", "middle_center") ATTDEF_TAG_RE = re.compile(r"^[A-Z][A-Z0-9_]{0,30}$") _REQUIRED = { "line": ("x1", "y1", "x2", "y2"), "circle": ("cx", "cy", "r"), "arc": ("cx", "cy", "r", "start_deg", "end_deg"), "polyline": ("points",), "text": ("text", "x", "y", "height"), "solid": ("points",), } _POSITIVE = {"r", "height"} def _scalar(value, where: str, *, positive: bool = False) -> float: if isinstance(value, bool) or not isinstance(value, (int, float)): raise TypeError(f"{where} must be a number, got {value!r}") number = float(value) if not math.isfinite(number): raise TypeError(f"{where} must be finite") if positive and number <= 0: raise TypeError(f"{where} must be > 0, got {number}") return number def _number(spec: dict, key: str, where: str) -> float: return _scalar(spec.get(key), f"{where}: {key!r}", positive=key in _POSITIVE) def _points( spec: dict, where: str, *, minimum: int, maximum: int | None = None ) -> list[tuple[float, float]]: raw = spec.get("points") if not isinstance(raw, (list, tuple)) or len(raw) < minimum: raise TypeError(f"{where}: 'points' must be a list of at least {minimum} [x, y] pairs") if maximum is not None and len(raw) > maximum: raise TypeError(f"{where}: 'points' must have at most {maximum} pairs") out: list[tuple[float, float]] = [] for i, pt in enumerate(raw): if not isinstance(pt, (list, tuple)) or len(pt) != 2: raise TypeError(f"{where}: points[{i}] must be an [x, y] pair") out.append( ( _scalar(pt[0], f"{where}: points[{i}].x"), _scalar(pt[1], f"{where}: points[{i}].y"), ) ) return out def _align(spec: dict, where: str) -> str: align = spec.get("align", "left") if align not in TEXT_ALIGNMENTS: raise TypeError(f"{where}: 'align' must be one of {TEXT_ALIGNMENTS}, got {align!r}") return align def _layer(spec: dict, where: str) -> str: layer = spec.get("layer", "0") if not isinstance(layer, str) or not layer.strip(): raise TypeError(f"{where}: 'layer' must be a non-empty string") return layer.strip() def validate_entity_specs(entities) -> list[dict]: """Normalise ``block_define`` primitive specs; refuse the whole list on the first defect.""" if not isinstance(entities, (list, tuple)) or not entities: raise ValueError( "block_define: 'entities' is empty — a block that draws nothing is refused" ) out: list[dict] = [] for i, spec in enumerate(entities): where = f"entities[{i}]" if not isinstance(spec, dict): raise TypeError(f"{where}: must be an object") kind = spec.get("type") if kind not in ALLOWED_TYPES: raise TypeError(f"{where}: 'type' must be one of {ALLOWED_TYPES}, got {kind!r}") for key in _REQUIRED[kind]: if key not in spec: raise TypeError(f"{where}: {kind} requires {key!r}") norm: dict = {"type": kind, "layer": _layer(spec, where)} if kind == "line": for key in ("x1", "y1", "x2", "y2"): norm[key] = _number(spec, key, where) elif kind == "circle": for key in ("cx", "cy", "r"): norm[key] = _number(spec, key, where) elif kind == "arc": for key in ("cx", "cy", "r", "start_deg", "end_deg"): norm[key] = _number(spec, key, where) elif kind == "polyline": pts = _points(spec, where, minimum=2) closed = spec.get("closed", False) if not isinstance(closed, bool): raise TypeError(f"{where}: 'closed' must be a boolean") bulges = spec.get("bulges") if bulges is not None: if not isinstance(bulges, (list, tuple)) or len(bulges) != len(pts): raise TypeError(f"{where}: 'bulges' must list one value per point") bulges = [_scalar(b, f"{where}: bulges[{j}]") for j, b in enumerate(bulges)] norm.update(points=pts, closed=closed, bulges=list(bulges) if bulges else None) elif kind == "text": text = spec.get("text") if not isinstance(text, str): raise TypeError(f"{where}: 'text' must be a string") norm.update( text=text, x=_number(spec, "x", where), y=_number(spec, "y", where), height=_number(spec, "height", where), rotation_deg=_scalar(spec.get("rotation_deg", 0.0), f"{where}: 'rotation_deg'"), align=_align(spec, where), ) elif kind == "solid": norm["points"] = _points(spec, where, minimum=3, maximum=4) out.append(norm) return out def validate_attdef_specs(attdefs) -> list[dict]: """Normalise ATTDEF specs: tag pattern, uniqueness, numeric fields, flags.""" if attdefs is None: return [] if not isinstance(attdefs, (list, tuple)): raise TypeError("block_define: 'attdefs' must be a list") out: list[dict] = [] seen: set[str] = set() for i, spec in enumerate(attdefs): where = f"attdefs[{i}]" if not isinstance(spec, dict): raise TypeError(f"{where}: must be an object") tag = spec.get("tag") if not isinstance(tag, str) or not ATTDEF_TAG_RE.match(tag): raise TypeError(f"{where}: 'tag' must match {ATTDEF_TAG_RE.pattern}, got {tag!r}") if tag in seen: raise TypeError(f"{where}: duplicate tag {tag!r}") seen.add(tag) prompt = spec.get("prompt", tag) default = spec.get("default", "") invisible = spec.get("invisible", False) if not isinstance(prompt, str): raise TypeError(f"{where}: 'prompt' must be a string") if not isinstance(default, str): raise TypeError(f"{where}: 'default' must be a string") if not isinstance(invisible, bool): raise TypeError(f"{where}: 'invisible' must be a boolean") out.append( { "tag": tag, "prompt": prompt, "default": default, "x": _number(spec, "x", where), "y": _number(spec, "y", where), "height": _number(spec, "height", where), "rotation_deg": _scalar(spec.get("rotation_deg", 0.0), f"{where}: 'rotation_deg'"), "align": _align(spec, where), "invisible": invisible, } ) return out def solid_vertices(points: list[tuple[float, float]]) -> list[tuple[float, float]]: """DXF SOLID draws vtx0→vtx1→vtx3→vtx2, so a quad given in polygon order (a, b, c, d) is stored as (a, b, d, c). Triangles are stored as given.""" pts = [(float(x), float(y)) for x, y in points] if len(pts) == 4: return [pts[0], pts[1], pts[3], pts[2]] return pts ``` - [ ] **Step 4: Run the tests** Run: `uv run --frozen pytest tests/test_block_specs.py -v` Expected: all PASS. - [ ] **Step 5: Commit** ```bash git add backends/block_specs.py tests/test_block_specs.py git commit -m "feat(blocks): shared validation for block_define primitive and ATTDEF specs" ``` --- ### Task 3: `block_define` on both engines + tool + gates **Files:** - Modify: `backends/contracts/blocks.py` (append abstract method) - Modify: `backends/ezdxf_backend.py` (after `block_create_from_entities`) - Modify: `backends/com_backend.py` (after `block_create_from_entities`) - Modify: `server.py` (SECTION 7 header `(7 tools)` → `(8 tools)`; new tool after `block_create_from_entities`) - Modify: `discovery/aliases.py` (record `block_define`) - Modify: `README.md` snapshot line `154 tools` → `155 tools`; regenerate `docs/tool-inventory.json` - Test: `tests/test_block_define.py` **Interfaces:** - Consumes: `validate_entity_specs`, `validate_attdef_specs`, `solid_vertices` (Task 2); `sanitize_symbol_name` (`security.py`). - Produces: `await backend.block_define(name, entities, attdefs=None, base_x=0.0, base_y=0.0, overwrite=False) -> {"ok": True, "name", "entity_count", "attdef_count", "replaced": bool, "backend"}`; MCP tool `block_define` with the same parameters. - [ ] **Step 1: Write the failing tests** ```python # tests/test_block_define.py """block_define: a definition with ATTDEFs from typed specs, on both engines.""" from __future__ import annotations import types import pytest pytestmark = pytest.mark.asyncio VALVE = [ {"type": "polyline", "points": [[-4, -2], [-4, 2], [0, 0]], "closed": True}, {"type": "polyline", "points": [[4, -2], [4, 2], [0, 0]], "closed": True}, {"type": "circle", "cx": 0, "cy": 0, "r": 1.5}, {"type": "arc", "cx": 0, "cy": 2, "r": 2, "start_deg": 0, "end_deg": 180}, {"type": "line", "x1": 0, "y1": 0, "x2": 0, "y2": 4}, {"type": "text", "text": "M", "x": 0, "y": 6, "height": 2, "align": "middle_center"}, {"type": "solid", "points": [[0, 0], [-3, 1.2], [-3, -1.2]]}, ] ATTDEFS = [ {"tag": "TAG", "x": 0, "y": 5.5, "height": 2.5, "align": "center"}, {"tag": "LINK", "x": 0, "y": -5, "height": 2.0, "invisible": True, "default": "L-1"}, ] # ── headless engine ───────────────────────────────────────────────────────── async def test_defines_geometry_and_attdefs(backend): result = await backend.block_define("PID_VALVE_TEST", VALVE, ATTDEFS) assert result == { "ok": True, "name": "PID_VALVE_TEST", "entity_count": 7, "attdef_count": 2, "replaced": False, "backend": "ezdxf", } blk = backend._doc.blocks.get("PID_VALVE_TEST") types_ = sorted(e.dxftype() for e in blk) assert types_ == [ "ARC", "ATTDEF", "ATTDEF", "CIRCLE", "LINE", "LWPOLYLINE", "LWPOLYLINE", "SOLID", "TEXT", ] attdefs = {e.dxf.tag: e for e in blk if e.dxftype() == "ATTDEF"} assert attdefs["LINK"].is_invisible is True assert attdefs["LINK"].dxf.text == "L-1" assert attdefs["TAG"].is_invisible is False for e in blk: assert e.dxf.layer == "0" assert e.dxf.color == 0, "ByBlock colour so the INSERT's layer drives it" async def test_insert_after_define_carries_the_attributes(backend): await backend.block_define("PID_VALVE_TEST", VALVE, ATTDEFS) ref = await backend.block_insert("PID_VALVE_TEST", 10, 10, attributes={"TAG": "HV-101"}) assert await backend.block_get_attributes(ref.handle) == {"TAG": "HV-101", "LINK": "L-1"} async def test_name_clash_is_refused_unless_overwrite(backend): await backend.block_define("PID_X", VALVE) with pytest.raises(ValueError, match="overwrite"): await backend.block_define("PID_X", VALVE) result = await backend.block_define("PID_X", VALVE[:1], overwrite=True) assert result["replaced"] is True and result["entity_count"] == 1 assert len(list(backend._doc.blocks.get("PID_X"))) == 1 async def test_overwrite_keeps_existing_inserts_pointing_at_the_name(backend): await backend.block_define("PID_X", VALVE) ref = await backend.block_insert("PID_X", 0, 0) await backend.block_define( "PID_X", [{"type": "circle", "cx": 0, "cy": 0, "r": 9}], overwrite=True ) info = await backend.entity_get(ref.handle) assert info.properties["block_name"] == "PID_X" virtual = list(backend._doc.entitydb[ref.handle].virtual_entities()) assert [e.dxftype() for e in virtual] == ["CIRCLE"] async def test_invalid_request_writes_nothing(backend): with pytest.raises(TypeError, match=r"entities\[1\]"): await backend.block_define("PID_BAD", [VALVE[0], {"type": "circle", "cx": 0, "cy": 0}]) assert "PID_BAD" not in backend._doc.blocks with pytest.raises(ValueError, match="empty"): await backend.block_define("PID_EMPTY", []) assert "PID_EMPTY" not in backend._doc.blocks async def test_anonymous_names_are_refused(backend): with pytest.raises(ValueError, match="anonymous"): await backend.block_define("*U9", VALVE) async def test_solid_uses_bowtie_order(backend): await backend.block_define( "PID_SOLID", [{"type": "solid", "points": [[0, 0], [1, 0], [1, 1], [0, 1]]}] ) solid = next(e for e in backend._doc.blocks.get("PID_SOLID") if e.dxftype() == "SOLID") assert tuple(solid.dxf.vtx2)[:2] == (0.0, 1.0) assert tuple(solid.dxf.vtx3)[:2] == (1.0, 1.0) # ── live engine, against a fake ActiveX surface ────────────────────────────── class _Recorder: """Records every method call made on it, returns a styled child.""" def __init__(self, name): self.Name = name self.calls: list[tuple[str, tuple]] = [] self.Count = 0 def __getattr__(self, attr): if attr.startswith("Add"): def _call(*args): self.calls.append((attr, args)) return types.SimpleNamespace() return _call raise AttributeError(attr) class _FakeBlocks: def __init__(self): self.blocks: dict[str, _Recorder] = {} def Add(self, base_point, name): self.blocks[name] = _Recorder(name) self.blocks[name].base_point = tuple(base_point.value) return self.blocks[name] def Item(self, name): if name not in self.blocks: raise RuntimeError(f"no block {name}") return self.blocks[name] @pytest.fixture def com_backend(monkeypatch): pytest.importorskip("win32com.client", reason="pywin32 not installed") from backends import com_backend as module document = types.SimpleNamespace(Blocks=_FakeBlocks()) monkeypatch.setattr(module, "_acad_doc", lambda: document) monkeypatch.setattr(module, "_regen", lambda: None) backend = module.ComBackend() async def _run_inline(func, *args, **kwargs): return func(*args, **kwargs) monkeypatch.setattr(backend, "_run", _run_inline) return backend, document async def test_com_issues_one_activex_call_per_primitive_and_attdef(com_backend): backend, document = com_backend result = await backend.block_define("PID_VALVE_TEST", VALVE, ATTDEFS, 1.0, 2.0) assert result["ok"] is True and result["backend"] == "com" and result["replaced"] is False block = document.Blocks.blocks["PID_VALVE_TEST"] assert block.base_point == (1.0, 2.0, 0.0) names = [name for name, _ in block.calls] assert names == [ "AddLightWeightPolyline", "AddLightWeightPolyline", "AddCircle", "AddArc", "AddLine", "AddText", "AddSolid", "AddAttribute", "AddAttribute", ] tag_call = block.calls[7][1] link_call = block.calls[8][1] assert tag_call[0] == 2.5 and tag_call[1] == 0 and tag_call[4] == "TAG" assert link_call[1] == 1, "invisible ATTDEF uses acAttributeModeInvisible" assert link_call[5] == "L-1" async def test_com_refuses_a_clash_without_overwrite(com_backend): backend, document = com_backend await backend.block_define("PID_X", VALVE[:1]) with pytest.raises(ValueError, match="overwrite"): await backend.block_define("PID_X", VALVE[:1]) async def test_com_validates_before_touching_activex(com_backend): backend, document = com_backend with pytest.raises(TypeError): await backend.block_define("PID_BAD", [{"type": "circle", "cx": 0}]) assert document.Blocks.blocks == {} ``` - [ ] **Step 2: Run them to verify they fail** Run: `uv run --frozen pytest tests/test_block_define.py -v` Expected: FAIL with `AttributeError: 'EzdxfBackend' object has no attribute 'block_define'`. - [ ] **Step 3: Add the contract method** Append to `BlockContract` in `backends/contracts/blocks.py`: ```python @abstractmethod async def block_define( self, name: str, entities: list[dict], attdefs: list[dict] | None = None, base_x: float = 0.0, base_y: float = 0.0, overwrite: bool = False, ) -> dict: """Create (or with ``overwrite`` replace the contents of) a block definition from typed primitive specs and ATTDEF specs — see ``backends/block_specs.py`` for the spec vocabulary. Returns ``{ok, name, entity_count, attdef_count, replaced, backend}``.""" ... ``` - [ ] **Step 4: Implement on ezdxf** Add after `block_create_from_entities` in `backends/ezdxf_backend.py`: ```python async def block_define( self, name, entities, attdefs=None, base_x=0.0, base_y=0.0, overwrite=False, ) -> dict: """A block definition with ATTDEFs from typed specs. Validation runs before any write, so a malformed entry leaves no definition behind. ``overwrite`` replaces the *contents* of an existing definition rather than deleting it: existing INSERTs keep pointing at the name and show the new geometry (deleting a referenced block is refused by both engines; replacing contents works on both). """ from ezdxf.enums import TextEntityAlignment from backends.block_specs import ( solid_vertices, validate_attdef_specs, validate_entity_specs, ) from security import sanitize_symbol_name clean_name = sanitize_symbol_name(name, kind="block") if clean_name.startswith("*"): raise ValueError("block_define: anonymous block names (*...) are refused") ents = validate_entity_specs(entities) atts = validate_attdef_specs(attdefs or []) align_map = { "left": TextEntityAlignment.LEFT, "center": TextEntityAlignment.CENTER, "right": TextEntityAlignment.RIGHT, "middle_center": TextEntityAlignment.MIDDLE_CENTER, } def _sync(): doc = self._require_doc() existing = doc.blocks.get(clean_name) if clean_name in doc.blocks else None if existing is not None and not overwrite: raise ValueError( f"block_define: block {clean_name!r} already exists; " "pass overwrite=true to replace its contents" ) replaced = existing is not None base = (float(base_x), float(base_y), 0.0) if existing is None: blk = doc.blocks.new(name=clean_name, base_point=base) else: blk = existing blk.delete_all_entities() blk.block.dxf.base_point = base for spec in ents: attribs = {"layer": spec["layer"], "color": 0, "linetype": "ByBlock"} kind = spec["type"] if kind == "line": blk.add_line((spec["x1"], spec["y1"]), (spec["x2"], spec["y2"]), dxfattribs=attribs) elif kind == "circle": blk.add_circle((spec["cx"], spec["cy"]), spec["r"], dxfattribs=attribs) elif kind == "arc": blk.add_arc( (spec["cx"], spec["cy"]), spec["r"], spec["start_deg"], spec["end_deg"], dxfattribs=attribs, ) elif kind == "polyline": bulges = spec["bulges"] or [0.0] * len(spec["points"]) blk.add_lwpolyline( [(x, y, b) for (x, y), b in zip(spec["points"], bulges)], format="xyb", close=spec["closed"], dxfattribs=attribs, ) elif kind == "text": txt = blk.add_text( spec["text"], dxfattribs={ **attribs, "height": spec["height"], "rotation": spec["rotation_deg"], }, ) txt.set_placement((spec["x"], spec["y"]), align=align_map[spec["align"]]) elif kind == "solid": blk.add_solid(solid_vertices(spec["points"]), dxfattribs=attribs) for att in atts: attdef = blk.add_attdef( att["tag"], insert=(att["x"], att["y"]), text=att["default"], dxfattribs={ "layer": "0", "color": 0, "height": att["height"], "rotation": att["rotation_deg"], "prompt": att["prompt"], }, ) attdef.set_placement((att["x"], att["y"]), align=align_map[att["align"]]) if att["invisible"]: attdef.is_invisible = True self._mark_dirty() return { "ok": True, "name": clean_name, "entity_count": len(ents), "attdef_count": len(atts), "replaced": replaced, "backend": "ezdxf", } return await self._async(_sync) ``` - [ ] **Step 5: Implement on COM** Add after `block_create_from_entities` in `backends/com_backend.py` (module already imports `deg2rad`, `win32com`, `pythoncom`, `_apoint`, `_av`, `_acad_doc`, `_regen`): ```python async def block_define( self, name, entities, attdefs=None, base_x=0.0, base_y=0.0, overwrite=False, ) -> dict: """A block definition with ATTDEFs from typed specs, through ActiveX. ``Blocks.Add`` creates the definition; each primitive is one ``Add*`` call on the Block object, each ATTDEF one ``AddAttribute``. Overwrite deletes the existing definition's members and re-adds — the name and its INSERTs survive. Unit-tested against a fake ActiveX surface; executed live by ``scripts/smoke_pid_com.py``. """ from backends.block_specs import ( solid_vertices, validate_attdef_specs, validate_entity_specs, ) from security import sanitize_symbol_name clean_name = sanitize_symbol_name(name, kind="block") if clean_name.startswith("*"): raise ValueError("block_define: anonymous block names (*...) are refused") ents = validate_entity_specs(entities) atts = validate_attdef_specs(attdefs or []) # acAlignmentLeft / Center / Right / MiddleCenter align_map = {"left": 0, "center": 1, "right": 2, "middle_center": 10} def _style(obj, layer: str) -> None: obj.Layer = layer obj.Color = 0 # acByBlock obj.Linetype = "ByBlock" def _sync(): doc = _acad_doc() try: existing = doc.Blocks.Item(clean_name) except Exception: existing = None if existing is not None and not overwrite: raise ValueError( f"block_define: block {clean_name!r} already exists; " "pass overwrite=true to replace its contents" ) replaced = existing is not None base = _apoint(float(base_x), float(base_y), 0.0) if existing is None: block = doc.Blocks.Add(base, clean_name) else: block = existing members = [block.Item(i) for i in range(block.Count)] for member in members: member.Delete() block.Origin = base for spec in ents: kind = spec["type"] if kind == "line": obj = block.AddLine( _apoint(spec["x1"], spec["y1"]), _apoint(spec["x2"], spec["y2"]) ) elif kind == "circle": obj = block.AddCircle(_apoint(spec["cx"], spec["cy"]), float(spec["r"])) elif kind == "arc": obj = block.AddArc( _apoint(spec["cx"], spec["cy"]), float(spec["r"]), deg2rad(spec["start_deg"]), deg2rad(spec["end_deg"]), ) elif kind == "polyline": flat = [coord for pt in spec["points"] for coord in pt] obj = block.AddLightWeightPolyline(_av(flat)) obj.Closed = bool(spec["closed"]) for index, bulge in enumerate(spec["bulges"] or []): if bulge: obj.SetBulge(index, float(bulge)) elif kind == "text": obj = block.AddText( spec["text"], _apoint(spec["x"], spec["y"]), float(spec["height"]) ) obj.Rotation = deg2rad(spec["rotation_deg"]) if spec["align"] != "left": obj.Alignment = align_map[spec["align"]] obj.TextAlignmentPoint = _apoint(spec["x"], spec["y"]) elif kind == "solid": verts = solid_vertices(spec["points"]) verts = verts + [verts[-1]] * (4 - len(verts)) obj = block.AddSolid(*[_apoint(x, y) for x, y in verts]) _style(obj, spec["layer"]) for att in atts: mode = 1 if att["invisible"] else 0 # acAttributeModeInvisible / Normal obj = block.AddAttribute( float(att["height"]), mode, att["prompt"], _apoint(att["x"], att["y"]), att["tag"], att["default"], ) obj.Rotation = deg2rad(att["rotation_deg"]) if att["align"] != "left": obj.Alignment = align_map[att["align"]] obj.TextAlignmentPoint = _apoint(att["x"], att["y"]) _style(obj, "0") _regen() return { "ok": True, "name": clean_name, "entity_count": len(ents), "attdef_count": len(atts), "replaced": replaced, "backend": "com", } return await self._run(_sync) ``` - [ ] **Step 6: Run the tests** Run: `uv run --frozen pytest tests/test_block_define.py tests/test_capability_contract.py -v` Expected: all PASS (the capability-contract test proves both backends still instantiate). - [ ] **Step 7: Register the tool, alias, header count, inventory, README** In `server.py`, change the header `# ── SECTION 7: Block Operations (7 tools)` to `(8 tools)` and add after `block_create_from_entities`: ```python @cad_tool( summary="Define a block from typed primitives and attribute definitions, on both engines.", cost="mutate", ) @mcp.tool( annotations={"title": "Define Block", "readOnlyHint": False}, tags={"block", "create"}, ) async def block_define( name: Annotated[str, "New block definition name"], entities: Annotated[ list[dict], "Primitives in block-local coordinates. type=line {x1,y1,x2,y2} | circle {cx,cy,r} | " "arc {cx,cy,r,start_deg,end_deg} | polyline {points,closed,bulges?} | " "text {text,x,y,height,rotation_deg?,align?} | solid {points (3-4)}. Optional layer (default 0).", ], attdefs: Annotated[ list[dict] | None, "Attribute definitions: {tag, x, y, height, prompt?, default?, rotation_deg?, align?, invisible?}", ] = None, base_x: Annotated[float, "Block base point X"] = 0.0, base_y: Annotated[float, "Block base point Y"] = 0.0, overwrite: Annotated[ bool, "Replace the contents of an existing definition of this name" ] = False, ctx: Context = None, ) -> dict: """Create a block definition with attribute definitions from typed specs. The whole request is validated before anything is written: one malformed entry refuses the call and leaves no definition behind. `overwrite=true` replaces the *contents* of an existing block so its INSERTs keep pointing at the name and show the new geometry; `replaced` reports it. Geometry inside the block is ByBlock so the INSERT's layer supplies colour and lineweight. Insert with `block_insert(attributes={TAG: value})`. """ await ctx.info(f"Defining block '{name}' from {len(entities)} primitives") return await _backend(ctx).block_define(name, entities, attdefs, base_x, base_y, overwrite) ``` In `discovery/aliases.py`, add to `TOOL_ALIASES` next to `block_create_from_entities`: ```python "block_define": ToolAliases( acad=("BLOCK", "BEDIT", "ATTDEF"), synonyms=( "define a block", "make a symbol", "block with attributes", "attribute definition", "create a block definition from scratch", ), ), ``` Then: `uv run --frozen python scripts/generate_tool_inventory.py` and change the README snapshot line to `155 tools`. - [ ] **Step 8: Run the gates** Run: `uv run --frozen pytest tests/test_block_define.py tests/test_discovery_aliases.py tests/test_tool_inventory.py tests/test_release_consistency.py tests/test_tool_registry.py -v` Expected: all PASS. (If `test_document_numbers_match_the_tree` fails only on the README `collected tests` figure, that drift predates this track — leave it for Task 21.) - [ ] **Step 9: Commit** ```bash git add backends/contracts/blocks.py backends/ezdxf_backend.py backends/com_backend.py server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_block_define.py git commit -m "feat(blocks): block_define — typed primitives + ATTDEFs on both engines" ``` --- ### Task 4: XDATA read/write on both engines + tools **Files:** - Create: `backends/xdata_specs.py` - Modify: `backends/contracts/entity_query.py` (append two abstract methods) - Modify: `backends/ezdxf_backend.py`, `backends/com_backend.py` - Modify: `server.py` (SECTION 5 header `(7 tools)` → `(9 tools)`; two tools after `entity_set_properties`) - Modify: `discovery/aliases.py`, `README.md` (`155 tools` → `157 tools`), regenerate `docs/tool-inventory.json` - Test: `tests/test_xdata.py` **Interfaces:** - Produces: - `backends/xdata_specs.py`: `validate_app_name(app) -> str`; `encode_values(values: list) -> list[tuple[int, object]]` (`str`→1000, `int`→1071, `float`→1040, `[x, y(, z)]`→1010 as `(x, y, z)`); `decode_values(tags) -> list` (inverse; 1010 → `[x, y, z]`); `split_by_app(codes, values) -> dict[str, list]`; `MAX_STRING = 255`, `MAX_BYTES = 16384`. - `await backend.entity_get_xdata(handle, app_name=None) -> {"handle", "xdata": {app: [values]}, "backend"}` - `await backend.entity_set_xdata(handle, app_name, values) -> {"ok": True, "handle", "app_name", "value_count", "bytes", "removed": bool, "backend"}` — empty `values` removes the app's XDATA. - [ ] **Step 1: Write the failing tests** ```python # tests/test_xdata.py """XDATA round trips, typing by group code, limits, removal — both engines.""" from __future__ import annotations import types import pytest from backends.xdata_specs import ( MAX_BYTES, MAX_STRING, decode_values, encode_values, split_by_app, validate_app_name, ) pytestmark = pytest.mark.asyncio # ── pure encoding ──────────────────────────────────────────────────────────── def test_values_are_typed_by_group_code(): assert encode_values(["json", 7, 2.5, [1, 2], [1, 2, 3]]) == [ (1000, "json"), (1071, 7), (1040, 2.5), (1010, (1.0, 2.0, 0.0)), (1010, (1.0, 2.0, 3.0)), ] def test_decode_is_the_inverse(): tags = encode_values(["a", 1, 1.5, [3, 4]]) assert decode_values(tags) == ["a", 1, 1.5, [3.0, 4.0, 0.0]] @pytest.mark.parametrize("bad", [True, None, {"a": 1}, [1], [1, 2, 3, 4], ["x", "y"], 2**31]) def test_untypeable_values_are_refused(bad): with pytest.raises((TypeError, ValueError)): encode_values([bad]) def test_string_over_255_is_refused(): encode_values(["x" * MAX_STRING]) with pytest.raises(ValueError, match="255"): encode_values(["x" * (MAX_STRING + 1)]) def test_total_size_over_16k_is_refused(): with pytest.raises(ValueError, match="16"): encode_values(["x" * 255] * (MAX_BYTES // 255 + 1)) @pytest.mark.parametrize("bad", ["", "1abc", "a-b", "a" * 32, "ACAD"]) def test_bad_app_names_are_refused(bad): with pytest.raises(ValueError): validate_app_name(bad) def test_split_by_app_groups_a_flat_activex_stream(): codes = [1001, 1000, 1071, 1001, 1040] values = ["APP_A", "hello", 3, "APP_B", 1.5] assert split_by_app(codes, values) == {"APP_A": ["hello", 3], "APP_B": [1.5]} # ── headless engine ───────────────────────────────────────────────────────── async def test_roundtrip_registers_the_appid(backend): line = await backend.entity_create_line(0, 0, 1, 1) result = await backend.entity_set_xdata(line.handle, "ACADMCP_PID", ["json", 42, 1.5, [1, 2]]) assert result["ok"] is True and result["value_count"] == 4 and result["removed"] is False assert "ACADMCP_PID" in backend._doc.appids got = await backend.entity_get_xdata(line.handle, "ACADMCP_PID") assert got["xdata"] == {"ACADMCP_PID": ["json", 42, 1.5, [1.0, 2.0, 0.0]]} async def test_get_without_app_returns_every_app(backend): line = await backend.entity_create_line(0, 0, 1, 1) await backend.entity_set_xdata(line.handle, "APP_A", ["a"]) await backend.entity_set_xdata(line.handle, "APP_B", ["b"]) got = await backend.entity_get_xdata(line.handle) assert got["xdata"] == {"APP_A": ["a"], "APP_B": ["b"]} async def test_unknown_app_is_empty_not_an_error(backend): line = await backend.entity_create_line(0, 0, 1, 1) got = await backend.entity_get_xdata(line.handle, "NOPE") assert got["xdata"] == {} async def test_set_replaces_and_empty_removes(backend): line = await backend.entity_create_line(0, 0, 1, 1) await backend.entity_set_xdata(line.handle, "APP_A", ["one", "two"]) await backend.entity_set_xdata(line.handle, "APP_A", ["three"]) assert (await backend.entity_get_xdata(line.handle, "APP_A"))["xdata"] == {"APP_A": ["three"]} result = await backend.entity_set_xdata(line.handle, "APP_A", []) assert result["removed"] is True assert (await backend.entity_get_xdata(line.handle))["xdata"] == {} async def test_limits_are_enforced_before_writing(backend): line = await backend.entity_create_line(0, 0, 1, 1) with pytest.raises(ValueError, match="255"): await backend.entity_set_xdata(line.handle, "APP_A", ["x" * 256]) assert (await backend.entity_get_xdata(line.handle))["xdata"] == {} # ── live engine, fake ActiveX ──────────────────────────────────────────────── class _FakeEntity: def __init__(self): self.stored: tuple | None = None def SetXData(self, types_, values): self.stored = (list(types_.value), list(values.value)) def GetXData(self, app): if self.stored is None: return ([], []) codes, values = self.stored if app: return (codes, values) return (codes, values) @pytest.fixture def com_backend(monkeypatch): pytest.importorskip("win32com.client", reason="pywin32 not installed") from backends import com_backend as module entity = _FakeEntity() registered: list[str] = [] document = types.SimpleNamespace( HandleToObject=lambda handle: entity, RegisteredApplications=types.SimpleNamespace(Add=registered.append), ) monkeypatch.setattr(module, "_acad_doc", lambda: document) backend = module.ComBackend() async def _run_inline(func, *args, **kwargs): return func(*args, **kwargs) monkeypatch.setattr(backend, "_run", _run_inline) return backend, entity, registered async def test_com_set_writes_the_1001_marker_first(com_backend): backend, entity, registered = com_backend await backend.entity_set_xdata("2F", "ACADMCP_PID", ["json", 3, [1, 2]]) assert registered == ["ACADMCP_PID"] codes, values = entity.stored assert codes == [1001, 1000, 1071, 1010] assert values[0] == "ACADMCP_PID" and values[1] == "json" and values[2] == 3 assert tuple(values[3].value) == (1.0, 2.0, 0.0) async def test_com_get_splits_by_app(com_backend): backend, entity, _ = com_backend await backend.entity_set_xdata("2F", "ACADMCP_PID", ["json"]) got = await backend.entity_get_xdata("2F", "ACADMCP_PID") assert got["xdata"] == {"ACADMCP_PID": ["json"]} async def test_com_empty_values_write_only_the_marker(com_backend): backend, entity, _ = com_backend result = await backend.entity_set_xdata("2F", "ACADMCP_PID", []) assert result["removed"] is True assert entity.stored == ([1001], ["ACADMCP_PID"]) ``` - [ ] **Step 2: Run them to verify they fail** Run: `uv run --frozen pytest tests/test_xdata.py -v` Expected: FAIL with `ModuleNotFoundError: No module named 'backends.xdata_specs'`. - [ ] **Step 3: Write `backends/xdata_specs.py`** ```python # backends/xdata_specs.py """XDATA value typing and limits, shared by both engines. DXF XDATA is a list of (group code, value) pairs under a registered APPID. This module is the one place that decides how a JSON value becomes a group code and enforces the two limits AutoCAD enforces silently: 255 characters per string and 16 KB per entity. """ from __future__ import annotations import re APP_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]{0,30}$") RESERVED_APPS = {"ACAD"} MAX_STRING = 255 MAX_BYTES = 16 * 1024 INT_MIN, INT_MAX = -(2**31), 2**31 - 1 def validate_app_name(app) -> str: if not isinstance(app, str) or not APP_NAME_RE.match(app): raise ValueError(f"xdata: app name must match {APP_NAME_RE.pattern}, got {app!r}") if app.upper() in RESERVED_APPS: raise ValueError(f"xdata: app name {app!r} is reserved") return app def encode_values(values) -> list[tuple[int, object]]: """JSON values → (group code, value). Raises TypeError/ValueError; writes nothing.""" if not isinstance(values, (list, tuple)): raise TypeError("xdata: values must be a list") tags: list[tuple[int, object]] = [] size = 0 for i, value in enumerate(values): where = f"values[{i}]" if isinstance(value, bool): raise TypeError(f"xdata: {where} booleans have no XDATA group code") if isinstance(value, str): if len(value) > MAX_STRING: raise ValueError( f"xdata: {where} is {len(value)} characters; the limit is {MAX_STRING}" ) tags.append((1000, value)) size += len(value.encode("utf-8")) + 2 elif isinstance(value, int): if not INT_MIN <= value <= INT_MAX: raise ValueError(f"xdata: {where} is outside the 32-bit integer range") tags.append((1071, int(value))) size += 6 elif isinstance(value, float): tags.append((1040, float(value))) size += 10 elif isinstance(value, (list, tuple)) and len(value) in (2, 3): try: coords = tuple(float(c) for c in value) except (TypeError, ValueError) as exc: raise TypeError(f"xdata: {where} point coordinates must be numbers") from exc if any(isinstance(c, bool) for c in value): raise TypeError(f"xdata: {where} point coordinates must be numbers") if len(coords) == 2: coords = (coords[0], coords[1], 0.0) tags.append((1010, coords)) size += 26 else: raise TypeError(f"xdata: {where} must be a string, int, float or [x, y(, z)] point") if size > MAX_BYTES: raise ValueError(f"xdata: encoded size {size} bytes exceeds the 16 KB per-entity limit") return tags def decode_values(tags) -> list: """(group code, value) → JSON values. Skips the 1001 app marker and 1002 braces.""" out: list = [] for code, value in tags: if code in (1001, 1002): continue if code == 1010 or code in (1011, 1012, 1013): out.append([float(c) for c in value]) elif code in (1040, 1041, 1042): out.append(float(value)) elif code in (1070, 1071): out.append(int(value)) else: out.append(str(value)) return out def split_by_app(codes, values) -> dict[str, list]: """A flat ActiveX GetXData stream (1001 markers inline) → {app: values}.""" out: dict[str, list] = {} current: str | None = None pending: list[tuple[int, object]] = [] for code, value in zip(codes, values): if code == 1001: if current is not None: out[current] = decode_values(pending) current, pending = str(value), [] else: pending.append((code, value)) if current is not None: out[current] = decode_values(pending) return out ``` - [ ] **Step 4: Add the contract methods** Append to the query contract class in `backends/contracts/entity_query.py`: ```python @abstractmethod async def entity_get_xdata(self, handle: str, app_name: str | None = None) -> dict: """Extended entity data as ``{"handle", "xdata": {app: [values]}, "backend"}``. ``app_name=None`` returns every app; an absent app is ``{}``, not an error.""" ... @abstractmethod async def entity_set_xdata(self, handle: str, app_name: str, values: list) -> dict: """Replace the app's XDATA (registering the APPID if needed). Values are typed by ``backends/xdata_specs.py``; an empty list removes the app's XDATA.""" ... ``` - [ ] **Step 5: Implement on ezdxf** (add near `block_get_attributes`) ```python async def entity_get_xdata(self, handle, app_name=None) -> dict: from backends.xdata_specs import decode_values, validate_app_name wanted = validate_app_name(app_name) if app_name else None def _sync(): ent = self._get_entity(handle) out: dict[str, list] = {} if ent.xdata is not None: apps = [wanted] if wanted else list(ent.xdata.data.keys()) for app in apps: if not ent.has_xdata(app): continue tags = ent.get_xdata(app) out[app] = decode_values((tag.code, tag.value) for tag in tags) return {"handle": handle, "xdata": out, "backend": "ezdxf"} return await self._async(_sync) async def entity_set_xdata(self, handle, app_name, values) -> dict: from backends.xdata_specs import encode_values, validate_app_name app = validate_app_name(app_name) tags = encode_values(values) def _sync(): doc = self._require_doc() ent = self._get_entity(handle) if not tags: ent.discard_xdata(app) self._mark_dirty() return { "ok": True, "handle": handle, "app_name": app, "value_count": 0, "bytes": 0, "removed": True, "backend": "ezdxf", } if app not in doc.appids: doc.appids.add(app) ent.set_xdata(app, tags) self._mark_dirty() return { "ok": True, "handle": handle, "app_name": app, "value_count": len(tags), "bytes": sum(len(str(v)) for _, v in tags), "removed": False, "backend": "ezdxf", } return await self._async(_sync) ``` - [ ] **Step 6: Implement on COM** (add near `block_get_attributes`) ```python async def entity_get_xdata(self, handle, app_name=None) -> dict: from backends.xdata_specs import split_by_app, validate_app_name wanted = validate_app_name(app_name) if app_name else "" def _sync(): doc = _acad_doc() ent = doc.HandleToObject(str(handle).strip().upper()) codes, values = ent.GetXData(wanted) grouped = split_by_app(list(codes or []), list(values or [])) if wanted: grouped = {k: v for k, v in grouped.items() if k == wanted} return {"handle": handle, "xdata": grouped, "backend": "com"} return await self._run(_sync) async def entity_set_xdata(self, handle, app_name, values) -> dict: from backends.xdata_specs import encode_values, validate_app_name app = validate_app_name(app_name) tags = encode_values(values) def _sync(): doc = _acad_doc() ent = doc.HandleToObject(str(handle).strip().upper()) try: doc.RegisteredApplications.Add(app) except Exception as exc: # already registered log.debug("RegisteredApplications.Add(%s): %s", app, exc) codes = [1001] + [code for code, _ in tags] payload = [app] + [_apoint(*value) if code == 1010 else value for code, value in tags] ent.SetXData( _ai(codes), win32com.client.VARIANT(pythoncom.VT_ARRAY | pythoncom.VT_VARIANT, payload), ) return { "ok": True, "handle": handle, "app_name": app, "value_count": len(tags), "bytes": sum(len(str(v)) for _, v in tags), "removed": not tags, "backend": "com", } return await self._run(_sync) ``` - [ ] **Step 7: Run the tests** Run: `uv run --frozen pytest tests/test_xdata.py tests/test_capability_contract.py -v` Expected: all PASS. - [ ] **Step 8: Tools, aliases, header, inventory, README** `server.py`: SECTION 5 header `(7 tools)` → `(9 tools)`; add after `entity_set_properties`: ```python @cad_tool(summary="Read an entity's extended data (XDATA) by application name.", cost="read") @mcp.tool( annotations={"title": "Get Entity XDATA", "readOnlyHint": True}, tags={"entity", "query"}, ) async def entity_get_xdata( handle: Annotated[str, "Entity handle"], app_name: Annotated[str | None, "Registered application name; omit for every app"] = None, ctx: Context = None, ) -> dict: """Extended entity data as {app: [values]}. An app with no XDATA is simply absent.""" return await _backend(ctx).entity_get_xdata(handle, app_name) @cad_tool(summary="Attach or replace extended data (XDATA) on an entity.", cost="safe") @mcp.tool( annotations={"title": "Set Entity XDATA", "readOnlyHint": False}, tags={"entity", "modify"}, ) async def entity_set_xdata( handle: Annotated[str, "Entity handle"], app_name: Annotated[str, "Registered application name (created if missing)"], values: Annotated[ list, "Values typed by kind: strings (≤255 chars) → 1000, ints → 1071, floats → 1040, [x,y] → 1010", ], ctx: Context = None, ) -> dict: """Replace the named app's XDATA on one entity; an empty list removes it. Limits are AutoCAD's and are enforced before writing: 255 characters per string and 16 KB per entity. """ return await _backend(ctx).entity_set_xdata(handle, app_name, values) ``` `discovery/aliases.py`, two records: ```python "entity_get_xdata": ToolAliases( acad=("XDATA", "XDLIST"), synonyms=("extended data", "read xdata", "entity metadata", "registered application data"), ), "entity_set_xdata": ToolAliases( acad=("XDATA",), synonyms=("attach extended data", "write xdata", "tag an entity with metadata"), ), ``` Regenerate the inventory; README snapshot `157 tools`. - [ ] **Step 9: Run the gates and commit** Run: `uv run --frozen pytest tests/test_xdata.py tests/test_discovery_aliases.py tests/test_tool_inventory.py tests/test_release_consistency.py -v` Expected: PASS (same caveat about the pre-existing collected-tests drift). ```bash git add backends/xdata_specs.py backends/contracts/entity_query.py backends/ezdxf_backend.py backends/com_backend.py server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_xdata.py git commit -m "feat(entity): entity_get_xdata / entity_set_xdata on both engines" ``` --- ### Task 5: `engineering/pid` package + `ACADMCP_PID` payloads **Files:** - Create: `engineering/pid/__init__.py`, `engineering/pid/xdata.py` - Test: `tests/test_pid_xdata.py` **Interfaces:** - Consumes: `backend.entity_get_xdata`, `backend.entity_set_xdata` (Task 4). - Produces (`engineering/pid/xdata.py`): `APP_NAME = "ACADMCP_PID"`, `MARKER = "json"`, `CHUNK = 255`, `encode_payload(payload: dict) -> list[str]`, `decode_payload(values: list) -> dict | None`, `async write_payload(backend, handle, payload) -> dict`, `async read_payload(backend, handle) -> dict | None`, `symbol_payload(spec, params=None) -> dict`, `line_payload(line_class, number, size, service, spec, insulation, seq, from_ref, to_ref) -> dict`, `marker_payload(line_handle) -> dict`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_xdata.py """ACADMCP_PID payloads survive the 255-character XDATA string limit.""" from __future__ import annotations import pytest from engineering.pid.xdata import ( APP_NAME, MARKER, decode_payload, encode_payload, line_payload, marker_payload, read_payload, write_payload, ) pytestmark = pytest.mark.asyncio def test_short_payload_is_marker_plus_one_chunk(): values = encode_payload({"v": 1, "kind": "marker", "line": "3A"}) assert values[0] == MARKER and len(values) == 2 assert decode_payload(values) == {"kind": "marker", "line": "3A", "v": 1} def test_long_payload_is_chunked_at_255_and_rejoined(): payload = {"v": 1, "kind": "symbol", "ports": {f"N{i}": [i, i, 90] for i in range(60)}} values = encode_payload(payload) assert all(len(v) <= 255 for v in values) assert len(values) > 2 assert decode_payload(values) == payload def test_decode_rejects_foreign_or_broken_values(): assert decode_payload([]) is None assert decode_payload(["not-json", "{}"]) is None assert decode_payload([MARKER, "{not json"]) is None def test_builders_carry_the_documented_keys(): line = line_payload( "process_major", "100-P-1001", "100", "P", "CS1", "IH", 1001, {"handle": "2F", "port": "discharge"}, {"handle": "31", "port": "N1"}, ) assert line["kind"] == "line" and line["class"] == "process_major" assert line["from"] == {"handle": "2F", "port": "discharge"} and line["seq"] == 1001 assert marker_payload("3A") == {"v": 1, "kind": "marker", "line": "3A"} async def test_roundtrip_through_a_backend(backend): line = await backend.entity_create_line(0, 0, 10, 0) payload = {"v": 1, "kind": "line", "class": "electric", "number": None} await write_payload(backend, line.handle, payload) assert await read_payload(backend, line.handle) == payload got = await backend.entity_get_xdata(line.handle, APP_NAME) assert got["xdata"][APP_NAME][0] == MARKER async def test_read_payload_is_none_without_xdata(backend): line = await backend.entity_create_line(0, 0, 10, 0) assert await read_payload(backend, line.handle) is None ``` - [ ] **Step 2: Run them to verify they fail** Run: `uv run --frozen pytest tests/test_pid_xdata.py -v` Expected: FAIL with `ModuleNotFoundError: No module named 'engineering.pid'`. - [ ] **Step 3: Write the package init and the module** ```python # engineering/pid/__init__.py """P&ID expert and reader: symbol catalogue, lines, ISA-5.1 tags, graph.""" ``` ```python # engineering/pid/xdata.py """``ACADMCP_PID`` XDATA payloads. XDATA strings are capped at 255 characters, so a JSON payload is stored as ``["json", chunk, chunk, ...]`` and re-joined on read. Every symbol INSERT and every P&ID line carries one, so a reader without the catalogue (another tool, a later catalogue version) still sees ports, classes and numbers. """ from __future__ import annotations import json from typing import TYPE_CHECKING, Any if TYPE_CHECKING: from backends.base import AutoCADBackend APP_NAME = "ACADMCP_PID" MARKER = "json" CHUNK = 255 PAYLOAD_VERSION = 1 def encode_payload(payload: dict) -> list[str]: text = json.dumps(payload, separators=(",", ":"), sort_keys=True, ensure_ascii=True) return [MARKER] + [text[i : i + CHUNK] for i in range(0, len(text), CHUNK)] def decode_payload(values: list) -> dict | None: if not values or values[0] != MARKER: return None parts = [v for v in values[1:] if isinstance(v, str)] try: decoded = json.loads("".join(parts)) except json.JSONDecodeError: return None return decoded if isinstance(decoded, dict) else None async def write_payload(backend: AutoCADBackend, handle: str, payload: dict) -> dict: return await backend.entity_set_xdata(handle, APP_NAME, encode_payload(payload)) async def read_payload(backend: AutoCADBackend, handle: str) -> dict | None: result = await backend.entity_get_xdata(handle, APP_NAME) return decode_payload(result.get("xdata", {}).get(APP_NAME, [])) def symbol_payload(spec: Any, params: dict | None = None) -> dict: """``spec`` is an ``engineering.pid.symbols.SymbolSpec`` (typed loosely to avoid an import cycle). Ports are symbol-local ``[x, y, direction, kind, radius]``.""" from .symbols import CATALOG_VERSION return { "v": PAYLOAD_VERSION, "kind": "symbol", "catalog": CATALOG_VERSION, "name": spec.name, "family": spec.family, "symbol": spec.symbol, "variant": spec.variant, "params": dict(params or spec.params), "ports": {p.name: [p.x, p.y, p.direction_deg, p.kind, p.radius] for p in spec.ports}, } def line_payload( line_class: str, number: str | None, size: str | None, service: str | None, spec: str | None, insulation: str | None, seq: int | None, from_ref: dict | None, to_ref: dict | None, ) -> dict: return { "v": PAYLOAD_VERSION, "kind": "line", "class": line_class, "number": number, "size": size, "service": service, "spec": spec, "insulation": insulation, "seq": seq, "from": from_ref, "to": to_ref, } def marker_payload(line_handle: str) -> dict: return {"v": PAYLOAD_VERSION, "kind": "marker", "line": line_handle} ``` - [ ] **Step 4: Run the tests (the `symbol_payload` import of `CATALOG_VERSION` is exercised in Task 7)** Run: `uv run --frozen pytest tests/test_pid_xdata.py -v` Expected: all PASS. - [ ] **Step 5: Commit** ```bash git add engineering/pid/__init__.py engineering/pid/xdata.py tests/test_pid_xdata.py git commit -m "feat(pid): ACADMCP_PID payload encoding with 255-char chunking" ``` --- ### Task 6: ISA-5.1 tag grammar (`engineering/pid/tags.py`) **Files:** - Create: `engineering/pid/tags.py`, `tests/data/isa51_tags.json` - Test: `tests/test_pid_tags.py` **Interfaces:** - Produces: `parse_tag(tag: str, kind: str = "auto", equipment_prefixes: dict | None = None) -> dict` (shape in spec §8), `split_instrument_tag(tag) -> tuple[str, str]` (`("FIC", "101A")`; area kept out, suffix kept in the loop text), `DEFAULT_EQUIPMENT_PREFIXES`, `FIRST_LETTERS`, `MODIFIERS`, `READOUT`, `OUTPUT`, `TRAILING`, `describe_tables() -> dict` (for the resource). - [ ] **Step 1: Write the fixture** ```json // tests/data/isa51_tags.json (JSON has no comments — drop this line when writing the file) { "valid": [ ["FIC-101", "Flow Indicating Controller", "101"], ["FT-101", "Flow Transmitter", "101"], ["FE-101", "Flow Element", "101"], ["FQI-102", "Flow Totalizing Indicator", "102"], ["FFC-103", "Flow Ratio Controller", "103"], ["FY-104", "Flow Relay", "104"], ["FCV-105", "Flow Control Valve", "105"], ["FIT-106", "Flow Indicating Transmitter", "106"], ["PT-201", "Pressure Transmitter", "201"], ["PI-201", "Pressure Indicator", "201"], ["PG-202", "Pressure Gauge", "202"], ["PDT-203", "Pressure Differential Transmitter", "203"], ["PDI-203", "Pressure Differential Indicator", "203"], ["PDIC-204", "Pressure Differential Indicating Controller", "204"], ["PSV-205", "Pressure Safety Valve", "205"], ["PSL-206", "Pressure Switch Low", "206"], ["PSHH-207", "Pressure Switch High-high", "207"], ["PCV-208", "Pressure Control Valve", "208"], ["PIC-209", "Pressure Indicating Controller", "209"], ["LT-301", "Level Transmitter", "301"], ["LG-302", "Level Gauge", "302"], ["LAHH-303", "Level Alarm High-high", "303"], ["LAL-304", "Level Alarm Low", "304"], ["LSLL-305", "Level Switch Low-low", "305"], ["LIC-306", "Level Indicating Controller", "306"], ["LCV-307", "Level Control Valve", "307"], ["LS-308", "Level Switch", "308"], ["TE-401", "Temperature Element", "401"], ["TW-402", "Temperature Well", "402"], ["TIC-403", "Temperature Indicating Controller", "403"], ["TSE-404", "Temperature Safety Element", "404"], ["TAH-405", "Temperature Alarm High", "405"], ["TT-406", "Temperature Transmitter", "406"], ["TCV-407", "Temperature Control Valve", "407"], ["AIT-501", "Analysis Indicating Transmitter", "501"], ["HV-601", "Hand Valve", "601"], ["HS-602", "Hand Switch", "602"], ["HIC-603", "Hand Indicating Controller", "603"], ["ZSO-701", "Position Switch Open", "701"], ["ZSC-702", "Position Switch Closed", "702"], ["ZT-703", "Position Transmitter", "703"], ["SIC-801", "Speed Indicating Controller", "801"], ["WIT-802", "Weight Indicating Transmitter", "802"], ["VT-803", "Vibration Transmitter", "803"], ["JI-804", "Power Indicator", "804"], ["EI-805", "Voltage Indicator", "805"], ["II-806", "Current Indicator", "806"], ["BE-807", "Burner Element", "807"], ["RE-808", "Radiation Element", "808"], ["UY-809", "Multivariable Relay", "809"], ["PL-810", "Pressure Light", "810"] ], "user_defined": [ ["XV-901", "Unclassified Valve", "901"], ["NV-902", "Valve", "902"], ["CIC-903", "Indicating Controller", "903"] ], "invalid": [ ["FCI-101", "I"], ["FIH-102", "H"], ["F-103", "function"], ["FICC-104", "repeated"], ["PSVX-105", "X"], ["ABCDEF-106", "D"], ["FQQ-107", "Q"], ["1FC", "format"], ["FIC", "format"], ["fic-101-", "format"], ["LSO-108", "O"] ], "formats": [ ["10-FIC-101A", "10", "FIC", "101", "A"], ["FIC 101", null, "FIC", "101", null], ["FIC101", null, "FIC", "101", null], ["fic-101b", null, "FIC", "101", "B"] ], "equipment": [ ["P-101A", "pump", "101", "A"], ["V-201", "vessel", "201", ""], ["E-301", "exchanger", "301", ""], ["10-K-401", "compressor", "401", ""] ] } ``` - [ ] **Step 2: Write the failing tests** ```python # tests/test_pid_tags.py """ISA-5.1 identification-letter grammar, pinned against a reader-checkable fixture.""" from __future__ import annotations import json from pathlib import Path import pytest from engineering.pid.tags import ( DEFAULT_EQUIPMENT_PREFIXES, describe_tables, parse_tag, split_instrument_tag, ) FIXTURE = json.loads((Path(__file__).parent / "data" / "isa51_tags.json").read_text("utf-8")) @pytest.mark.parametrize("tag, description, loop", FIXTURE["valid"]) def test_valid_tags_parse_with_the_standard_description(tag, description, loop): out = parse_tag(tag) assert out["valid"] is True, out["errors"] assert out["kind"] == "instrument" assert out["description"] == description assert out["loop"] == loop assert out["user_defined"] is False @pytest.mark.parametrize("tag, description, loop", FIXTURE["user_defined"]) def test_user_choice_letters_parse_as_user_defined(tag, description, loop): out = parse_tag(tag) assert out["valid"] is True and out["user_defined"] is True assert out["description"] == description @pytest.mark.parametrize("tag, fragment", FIXTURE["invalid"]) def test_invalid_tags_name_the_offending_letter(tag, fragment): out = parse_tag(tag, kind="instrument") assert out["valid"] is False assert out["tag"] == tag assert any(fragment in err for err in out["errors"]), out["errors"] @pytest.mark.parametrize("tag, area, letters, loop, suffix", FIXTURE["formats"]) def test_tag_formats(tag, area, letters, loop, suffix): out = parse_tag(tag) assert (out["area"], out["letters"], out["loop"], out["suffix"]) == ( area, letters, loop, suffix, ) @pytest.mark.parametrize("tag, kind, number, suffix", FIXTURE["equipment"]) def test_equipment_tags(tag, kind, number, suffix): out = parse_tag(tag) assert out["kind"] == "equipment" and out["valid"] is True assert out["equipment_kind"] == kind and out["number"] == number and out["suffix"] == suffix def test_equipment_prefixes_can_be_replaced_per_call(): out = parse_tag("Q-1", kind="equipment", equipment_prefixes={"Q": "quench tower"}) assert out["valid"] is True and out["equipment_kind"] == "quench tower" default = parse_tag("Q-1", kind="equipment") assert default["valid"] is False and "prefix" in default["errors"][0] assert DEFAULT_EQUIPMENT_PREFIXES["P"] == "pump" def test_auto_kind_prefers_instrument_then_equipment_then_reports_instrument_errors(): assert parse_tag("PT-1")["kind"] == "instrument" assert parse_tag("P-1")["kind"] == "equipment" bad = parse_tag("FCI-1") assert bad["kind"] == "instrument" and bad["valid"] is False def test_split_instrument_tag_feeds_func_and_loop_attributes(): assert split_instrument_tag("10-FIC-101A") == ("FIC", "101A") assert split_instrument_tag("PT 7") == ("PT", "7") assert split_instrument_tag("garbage") == ("GARBAGE", "") def test_describe_tables_is_json_serialisable_and_names_every_first_letter(): tables = describe_tables() json.dumps(tables) assert set(tables["first_letters"]) == set("ABCDEFGHIJKLMNOPQRSTUVWXYZ") ``` - [ ] **Step 3: Run them to verify they fail** Run: `uv run --frozen pytest tests/test_pid_tags.py -v` Expected: FAIL with `ModuleNotFoundError: No module named 'engineering.pid.tags'`. - [ ] **Step 4: Write the module** ```python # engineering/pid/tags.py """ISA-5.1-2009 identification letters (Table 4.1) as a deterministic grammar. tag := first modifier? readout* output* trailing? modifier := D | F | K | Q | S readout := A | B | E | G | I | L | N | O | P | R | U | W | X output := C | K | S | T | U | V | X | Y | Z trailing := HH | LL | H | L | M | O | C The disambiguation rules (safety S vs switch S, trailing L vs light L, ZSO/ZSC, ratio F only after F, rate-of-change K only before an output) are the ones in the design spec; the fixture ``tests/data/isa51_tags.json`` pins every one of them against the standard's own examples. """ from __future__ import annotations import re #: First letter → measured/initiating variable. ``None`` = user's choice. FIRST_LETTERS: dict[str, str | None] = { "A": "Analysis", "B": "Burner", "C": None, "D": None, "E": "Voltage", "F": "Flow", "G": None, "H": "Hand", "I": "Current", "J": "Power", "K": "Time", "L": "Level", "M": None, "N": None, "O": None, "P": "Pressure", "Q": "Quantity", "R": "Radiation", "S": "Speed", "T": "Temperature", "U": "Multivariable", "V": "Vibration", "W": "Weight", "X": "Unclassified", "Y": "Event", "Z": "Position", } MODIFIERS: dict[str, str] = { "D": "Differential", "F": "Ratio", "K": "Rate of change", "Q": "Totalizing", "S": "Safety", } #: Readout / passive function letters → (adjective form, noun form). ``None`` = user's choice. READOUT: dict[str, tuple[str, str] | None] = { "A": ("Alarm", "Alarm"), "B": None, "E": ("Element", "Element"), "G": ("Gauge", "Gauge"), "I": ("Indicating", "Indicator"), "L": ("Light", "Light"), "N": None, "O": ("Orifice", "Orifice"), "P": ("Test point", "Test point"), "R": ("Recording", "Recorder"), "U": ("Multifunction", "Multifunction"), "W": ("Well", "Well"), "X": None, } #: Output / active function letters → word. ``None`` = user's choice. OUTPUT: dict[str, str | None] = { "C": "Controller", "K": "Control station", "S": "Switch", "T": "Transmitter", "U": "Multifunction", "V": "Valve", "X": None, "Y": "Relay", "Z": "Driver", } TRAILING: dict[str, str] = { "HH": "High-high", "LL": "Low-low", "H": "High", "L": "Low", "M": "Middle", "O": "Open", "C": "Closed", } DEFAULT_EQUIPMENT_PREFIXES: dict[str, str] = { "P": "pump", "V": "vessel", "T": "tank", "E": "exchanger", "K": "compressor", "C": "column", "R": "reactor", "F": "filter", "D": "drum", "M": "mixer", "H": "heater", "S": "separator", "B": "blower", "X": "package", } _INSTRUMENT_RE = re.compile( r"^(?:(?P\d{1,4})-)?(?P[A-Z]{1,5})[- ]?(?P\d{1,5})(?:[- ]?(?P[A-Z]))?$" ) _EQUIPMENT_RE = re.compile( r"^(?:(?P\d{1,4})-)?(?P[A-Z]{1,3})-(?P\d{1,5})(?P[A-Z]{0,2})$" ) def _normalise(tag) -> str: return str(tag or "").strip().upper() def _parse_letters(letters: str) -> dict: """Split ``letters`` into first/modifier/readout/output/trailing or report the first error.""" errors: list[str] = [] first = letters[0] i, n = 1, len(letters) if n == 1: return {"errors": [f"{letters}: a tag needs at least one function letter after {first}"]} modifier = None if letters[i] in MODIFIERS: candidate = letters[i] following = letters[i + 1] if i + 1 < n else "" take = ( candidate in ("D", "Q") or (candidate == "S" and following in ("V", "E")) or (candidate == "F" and first == "F") or (candidate == "K" and following in OUTPUT) ) if take: modifier = candidate i += 1 readout: list[str] = [] while i < n and letters[i] in READOUT: letter = letters[i] if letter in ("H", "L", "M") and readout and readout[-1] == "A": break # trailing high/low/middle after an alarm if readout and readout[-1] == letter: errors.append(f"{letters}: repeated letter {letter} at position {i + 1}") return {"errors": errors} readout.append(letter) i += 1 output: list[str] = [] while i < n and letters[i] in OUTPUT: letter = letters[i] if letter == "C" and output and output[-1] == "S" and i == n - 1: break # ZSC: closed, not controller if output and output[-1] == letter: errors.append(f"{letters}: repeated letter {letter} at position {i + 1}") return {"errors": errors} output.append(letter) i += 1 trailing = letters[i:] if trailing: if trailing not in TRAILING: errors.append(f"{letters}: unexpected letter {trailing[0]} at position {i + 1}") return {"errors": errors} after_alarm = bool(readout) and readout[-1] == "A" and not output after_switch = bool(output) and output[-1] == "S" if trailing in ("O", "C") and not after_switch: errors.append( f"{letters}: trailing {trailing} (open/closed) is only valid after a switch" ) return {"errors": errors} if trailing in ("H", "L", "M", "HH", "LL") and not (after_alarm or after_switch): errors.append( f"{letters}: trailing {trailing} needs an alarm (A) or switch (S) before it" ) return {"errors": errors} return { "first": first, "modifier": modifier, "readout": readout, "output": output, "trailing": trailing or None, "errors": [], } def _describe(parts: dict) -> tuple[str, bool]: words: list[str] = [] user_defined = False first_word = FIRST_LETTERS[parts["first"]] if first_word is None: user_defined = True else: words.append(first_word) if parts["modifier"]: words.append(MODIFIERS[parts["modifier"]]) readout, output = parts["readout"], parts["output"] for index, letter in enumerate(readout): forms = READOUT[letter] if forms is None: user_defined = True continue last_function = index == len(readout) - 1 and not output words.append(forms[1] if last_function else forms[0]) for index, letter in enumerate(output): word = OUTPUT[letter] if word is None: user_defined = True continue if letter == "C" and index + 1 < len(output) and output[index + 1] == "V": word = "Control" words.append(word) if parts["trailing"]: words.append(TRAILING[parts["trailing"]]) return " ".join(words), user_defined def _parse_instrument(raw: str) -> dict: out = { "tag": raw, "valid": False, "kind": "instrument", "area": None, "letters": None, "first": None, "modifier": None, "readout": [], "output": [], "trailing": None, "loop": None, "suffix": None, "description": None, "user_defined": False, "errors": [], } tag = _normalise(raw) match = _INSTRUMENT_RE.match(tag) if not match: out["errors"] = [f"{raw!r}: not in [area-]LETTERS[-]loop[suffix] format"] return out out.update( area=match.group("area"), letters=match.group("letters"), loop=match.group("loop"), suffix=match.group("suffix"), ) parts = _parse_letters(match.group("letters")) if parts["errors"]: out["errors"] = parts["errors"] return out description, user_defined = _describe(parts) out.update( valid=True, first={"letter": parts["first"], "meaning": FIRST_LETTERS[parts["first"]]}, modifier=( {"letter": parts["modifier"], "meaning": MODIFIERS[parts["modifier"]]} if parts["modifier"] else None ), readout=[ {"letter": l, "meaning": (READOUT[l] or (None, None))[1]} for l in parts["readout"] ], output=[{"letter": l, "meaning": OUTPUT[l]} for l in parts["output"]], trailing=parts["trailing"], description=description, user_defined=user_defined, ) return out def _parse_equipment(raw: str, prefixes: dict[str, str]) -> dict: out = { "tag": raw, "valid": False, "kind": "equipment", "area": None, "prefix": None, "equipment_kind": None, "number": None, "suffix": "", "errors": [], } match = _EQUIPMENT_RE.match(_normalise(raw)) if not match: out["errors"] = [f"{raw!r}: not in [area-]PREFIX-number[suffix] format"] return out prefix = match.group("prefix") out.update( area=match.group("area"), prefix=prefix, number=match.group("number"), suffix=match.group("suffix") or "", ) kind = prefixes.get(prefix) if kind is None: out["errors"] = [f"{raw!r}: prefix {prefix} is not in the equipment prefix map"] return out out.update(valid=True, equipment_kind=kind) return out def parse_tag(tag: str, kind: str = "auto", equipment_prefixes: dict | None = None) -> dict: """Parse an ISA-5.1 instrument tag or an equipment tag. ``kind``: ``"instrument"``, ``"equipment"`` or ``"auto"`` (instrument first, then equipment, else the instrument parse with its errors). """ prefixes = {k.upper(): v for k, v in (equipment_prefixes or DEFAULT_EQUIPMENT_PREFIXES).items()} if kind == "instrument": return _parse_instrument(tag) if kind == "equipment": return _parse_equipment(tag, prefixes) if kind != "auto": raise ValueError("kind must be 'auto', 'instrument' or 'equipment'") instrument = _parse_instrument(tag) if instrument["valid"]: return instrument equipment = _parse_equipment(tag, prefixes) return equipment if equipment["valid"] else instrument def split_instrument_tag(tag: str) -> tuple[str, str]: """``FUNC`` and ``LOOP`` attribute texts for a bubble: letters, and loop+suffix. Anything unparseable goes wholesale into ``FUNC`` so nothing is dropped.""" match = _INSTRUMENT_RE.match(_normalise(tag)) if not match: return _normalise(tag), "" return match.group("letters"), match.group("loop") + (match.group("suffix") or "") def describe_tables() -> dict: """The letter tables, for the ``autocad://standards/isa51`` resource.""" return { "standard": "ISA-5.1-2009 Table 4.1", "grammar": "first modifier? readout* output* trailing?", "first_letters": {k: v or "user's choice" for k, v in FIRST_LETTERS.items()}, "modifiers": dict(MODIFIERS), "readout": {k: (v[1] if v else "user's choice") for k, v in READOUT.items()}, "output": {k: v or "user's choice" for k, v in OUTPUT.items()}, "trailing": dict(TRAILING), "equipment_prefixes": dict(DEFAULT_EQUIPMENT_PREFIXES), "rules": [ "S in second position is the safety modifier only before V or E (PSV, TSE); otherwise a switch.", "D in second position is always differential.", "F in second position is ratio only when the first letter is F (FFC).", "K in second position is rate of change only before an output letter.", "H/L/M after an alarm (A) or switch (S) are trailing high/low/middle; L elsewhere is a light.", "O/C as the last letter after a switch are open/closed (ZSO, ZSC).", ], } ``` - [ ] **Step 5: Run the tests** Run: `uv run --frozen pytest tests/test_pid_tags.py -v` Expected: all PASS. If a fixture row fails, check the row against ISA-5.1 Table 4.1 before touching the grammar — the fixture is the reader's contract. - [ ] **Step 6: Commit** ```bash git add engineering/pid/tags.py tests/data/isa51_tags.json tests/test_pid_tags.py git commit -m "feat(pid): ISA-5.1 identification-letter grammar with a pinned fixture" ``` --- ### Task 7: Symbol catalogue framework, instruments, markers, connectors **Files:** - Create: `engineering/pid/geometry.py`, `engineering/pid/symbols.py` - Test: `tests/test_pid_symbols.py` **Interfaces:** - Produces (`geometry.py`): `line(x1,y1,x2,y2)`, `circle(cx,cy,r)`, `arc(cx,cy,r,start_deg,end_deg)`, `polyline(points, closed=True, bulges=None)`, `text(t,x,y,height,align="middle_center",rotation_deg=0.0)`, `solid(points)`, `dashed_line(x1,y1,x2,y2,on=1.0,off=1.0) -> list[dict]`, `regular_polygon(cx,cy,r,n,start_deg=0.0) -> list[tuple]`, `attdef(tag,x,y,height,align="center",invisible=False,default="")`, `bbox_of(primitives) -> tuple`. - Produces (`symbols.py`): `Port`, `SymbolSpec`, `CATALOG_VERSION = "1"`, `FAMILY_LAYER`, `INSTRUMENT_TYPES`, `INSTRUMENT_LOCATIONS`, `BUBBLE_RADIUS = 5.0`, `register(symbol, family, builder, variants=())`, `resolve(symbol, **options) -> SymbolSpec`, `list_symbols(family=None) -> list[dict]`, `all_specs() -> list[SymbolSpec]`, `transform_port(port, x, y, rotation_deg, scale) -> dict`, `build_instrument(type, location)`, `build_marker(which)`, `build_connector(which, direction="out")`, `tag_attdefs(ymax, with_desc=True)`. - Later tasks add builders by importing `register` from this module (`symbols_valves.py`, `symbols_equipment.py`, `symbols_vessels.py`) and `symbols.py` imports them at the bottom so `resolve` sees the whole catalogue. - [ ] **Step 1: Write the failing tests (the harness parametrises over `all_specs()`, so Tasks 8–10 extend coverage without new test code)** ```python # tests/test_pid_symbols.py """Every catalogue symbol: valid primitives, honest bbox, ports on the boundary.""" from __future__ import annotations import math import re import pytest from backends.block_specs import validate_attdef_specs, validate_entity_specs from engineering.pid import symbols from engineering.pid.geometry import bbox_of from engineering.pid.symbols import ( BUBBLE_RADIUS, CATALOG_VERSION, INSTRUMENT_LOCATIONS, INSTRUMENT_TYPES, Port, all_specs, list_symbols, resolve, transform_port, ) NAME_RE = re.compile(r"^PID_[A-Z0-9_]+$") SOURCE_RE = re.compile(r"^(ISO 10628-2|ISA-5\.1|PIP PIC001)") EPS = 1e-6 def _ids(): return [spec.name for spec in all_specs()] @pytest.mark.parametrize("spec", all_specs(), ids=_ids()) def test_primitives_and_attdefs_validate(spec): validate_entity_specs(list(spec.primitives)) validate_attdef_specs(list(spec.attdefs)) @pytest.mark.parametrize("spec", all_specs(), ids=_ids()) def test_declared_bbox_contains_every_primitive(spec): xmin, ymin, xmax, ymax = bbox_of(spec.primitives) dxmin, dymin, dxmax, dymax = spec.bbox assert dxmin - EPS <= xmin and dymin - EPS <= ymin assert xmax <= dxmax + EPS and ymax <= dymax + EPS assert dxmax > dxmin and dymax > dymin @pytest.mark.parametrize("spec", all_specs(), ids=_ids()) def test_ports_sit_on_the_boundary_or_at_the_centre(spec): if spec.family == "marker": assert spec.ports == () return assert spec.ports, "every non-marker symbol needs at least one port" xmin, ymin, xmax, ymax = spec.bbox for port in spec.ports: if port.direction_deg is None: assert port.radius > 0 and (port.x, port.y) == (0.0, 0.0) continue on_edge = ( abs(port.x - xmin) < EPS or abs(port.x - xmax) < EPS or abs(port.y - ymin) < EPS or abs(port.y - ymax) < EPS ) assert on_edge, ( f"{spec.name}.{port.name} at ({port.x}, {port.y}) is inside bbox {spec.bbox}" ) assert port.direction_deg in (0.0, 90.0, 180.0, 270.0) assert port.kind in ("process", "signal") @pytest.mark.parametrize("spec", all_specs(), ids=_ids()) def test_naming_source_and_layer(spec): assert NAME_RE.match(spec.name) assert SOURCE_RE.match(spec.source), spec.source assert spec.layer_class in ("PROCESS-VALVES", "PROCESS-EQUIPMENT", "INSTRUMENT-SYMBOL", "line") tags = [a["tag"] for a in spec.attdefs] assert len(tags) == len(set(tags)) for prim in spec.primitives: assert prim.get("layer", "0") == "0" def test_catalogue_names_are_unique_and_versioned(): names = [s.name for s in all_specs()] assert len(names) == len(set(names)) assert CATALOG_VERSION == "1" def test_instrument_generator_covers_four_types_by_five_locations(): specs = { (t, loc): resolve("instrument", type=t, location=loc) for t in INSTRUMENT_TYPES for loc in INSTRUMENT_LOCATIONS } assert len(specs) == 20 field = specs[("discrete", "field")] assert field.name == "PID_INST_DISCRETE_FIELD" assert [a["tag"] for a in field.attdefs] == ["FUNC", "LOOP"] assert field.ports == (Port("signal", 0.0, 0.0, None, "signal", BUBBLE_RADIUS),) assert sum(1 for p in field.primitives if p["type"] == "circle") == 1 primary = specs[("discrete", "primary")] assert sum(1 for p in primary.primitives if p["type"] == "line") == 1 aux_rear = specs[("plc", "auxiliary_rear")] assert sum(1 for p in aux_rear.primitives if p["type"] == "line") > 2, "dashed pair" assert sum(1 for p in aux_rear.primitives if p["type"] == "polyline") == 2, "square + diamond" hexagon = next(p for p in specs[("computer", "field")].primitives if p["type"] == "polyline") assert len(hexagon["points"]) == 6 def test_unknown_symbol_or_option_names_the_valid_values(): with pytest.raises(ValueError, match="instrument"): resolve("thermometer") with pytest.raises(ValueError, match="field"): resolve("instrument", type="dcs", location="ceiling") def test_markers_and_connectors(): arrow = resolve("arrow_flow") assert arrow.family == "marker" and arrow.layer_class == "line" assert any(p["type"] == "solid" for p in arrow.primitives) for which in ("mark_pneumatic", "mark_capillary", "mark_hydraulic", "mark_data"): assert resolve(which).family == "marker" out = resolve("offpage", direction="out") inn = resolve("offpage", direction="in") assert out.name == "PID_CONNECTOR_OFFPAGE_OUT" and inn.name == "PID_CONNECTOR_OFFPAGE_IN" assert out.ports[0] == Port("process", -8.0, 0.0, 180.0, "process") assert inn.ports[0] == Port("process", 8.0, 0.0, 0.0, "process") assert [a["tag"] for a in out.attdefs] == ["TAG", "LINK"] assert next(a for a in out.attdefs if a["tag"] == "LINK")["invisible"] is True def test_transform_port_applies_rotation_and_scale_about_the_insertion(): port = Port("out", 4.0, 0.0, 0.0, "process") moved = transform_port(port, 100.0, 50.0, 90.0, 2.0) assert moved["x"] == pytest.approx(100.0) and moved["y"] == pytest.approx(58.0) assert moved["direction_deg"] == pytest.approx(90.0) radial = transform_port(Port("signal", 0.0, 0.0, None, "signal", 5.0), 10.0, 10.0, 45.0, 3.0) assert radial["direction_deg"] is None and radial["radius"] == pytest.approx(15.0) def test_list_symbols_reports_variants_and_ports(): rows = {row["symbol"]: row for row in list_symbols()} assert rows["instrument"]["variants"] == { "type": list(INSTRUMENT_TYPES), "location": list(INSTRUMENT_LOCATIONS), } assert rows["offpage"]["variants"] == {"direction": ["in", "out"]} assert rows["arrow_flow"]["ports"] == [] assert list_symbols(family="marker") and all( r["family"] == "marker" for r in list_symbols(family="marker") ) ``` - [ ] **Step 2: Run them to verify they fail** Run: `uv run --frozen pytest tests/test_pid_symbols.py -v` Expected: FAIL with `ModuleNotFoundError`. - [ ] **Step 3: Write `geometry.py`** ```python # engineering/pid/geometry.py """Primitive-spec helpers for authoring symbols (block-local millimetres).""" from __future__ import annotations import math def line(x1: float, y1: float, x2: float, y2: float) -> dict: return {"type": "line", "x1": float(x1), "y1": float(y1), "x2": float(x2), "y2": float(y2)} def circle(cx: float, cy: float, r: float) -> dict: return {"type": "circle", "cx": float(cx), "cy": float(cy), "r": float(r)} def arc(cx: float, cy: float, r: float, start_deg: float, end_deg: float) -> dict: return { "type": "arc", "cx": float(cx), "cy": float(cy), "r": float(r), "start_deg": float(start_deg), "end_deg": float(end_deg), } def polyline(points, closed: bool = True, bulges=None) -> dict: spec = { "type": "polyline", "points": [[float(x), float(y)] for x, y in points], "closed": closed, } if bulges is not None: spec["bulges"] = [float(b) for b in bulges] return spec def text( t: str, x: float, y: float, height: float, align: str = "middle_center", rotation_deg: float = 0.0, ) -> dict: return { "type": "text", "text": t, "x": float(x), "y": float(y), "height": float(height), "align": align, "rotation_deg": float(rotation_deg), } def solid(points) -> dict: return {"type": "solid", "points": [[float(x), float(y)] for x, y in points]} def dashed_line( x1: float, y1: float, x2: float, y2: float, on: float = 1.0, off: float = 1.0 ) -> list[dict]: """A dashed line as short LINE primitives, so the block needs no linetype.""" length = math.hypot(x2 - x1, y2 - y1) ux, uy = (x2 - x1) / length, (y2 - y1) / length out: list[dict] = [] pos = 0.0 while pos < length - 1e-9: end = min(pos + on, length) out.append(line(x1 + ux * pos, y1 + uy * pos, x1 + ux * end, y1 + uy * end)) pos = end + off return out def regular_polygon( cx: float, cy: float, r: float, n: int, start_deg: float = 0.0 ) -> list[tuple[float, float]]: return [ ( cx + r * math.cos(math.radians(start_deg + 360.0 * i / n)), cy + r * math.sin(math.radians(start_deg + 360.0 * i / n)), ) for i in range(n) ] def attdef( tag: str, x: float, y: float, height: float, align: str = "center", invisible: bool = False, default: str = "", ) -> dict: return { "tag": tag, "prompt": tag, "default": default, "x": float(x), "y": float(y), "height": float(height), "align": align, "invisible": invisible, } def _arc_extent(cx, cy, r, start_deg, end_deg): pts = [] sweep = (end_deg - start_deg) % 360.0 or 360.0 for k in range(0, int(sweep) + 1): a = math.radians(start_deg + k) pts.append((cx + r * math.cos(a), cy + r * math.sin(a))) a = math.radians(end_deg) pts.append((cx + r * math.cos(a), cy + r * math.sin(a))) return pts def bbox_of(primitives) -> tuple[float, float, float, float]: """Extent of the drawn geometry (text counts by its insertion point only).""" xs: list[float] = [] ys: list[float] = [] for p in primitives: kind = p["type"] if kind == "line": xs += [p["x1"], p["x2"]] ys += [p["y1"], p["y2"]] elif kind == "circle": xs += [p["cx"] - p["r"], p["cx"] + p["r"]] ys += [p["cy"] - p["r"], p["cy"] + p["r"]] elif kind == "arc": for x, y in _arc_extent(p["cx"], p["cy"], p["r"], p["start_deg"], p["end_deg"]): xs.append(x) ys.append(y) elif kind in ("polyline", "solid"): for x, y in p["points"]: xs.append(x) ys.append(y) elif kind == "text": xs.append(p["x"]) ys.append(p["y"]) if not xs: return (0.0, 0.0, 0.0, 0.0) return (min(xs), min(ys), max(xs), max(ys)) ``` - [ ] **Step 4: Write `symbols.py`** ```python # engineering/pid/symbols.py """The P&ID symbol catalogue: symbols as data, blocks on demand. Every symbol is a ``SymbolSpec`` — typed primitives in symbol-local mm, ATTDEFs, named ports with an outward direction, a declared bbox — authored from ISO 10628-2 (equipment, valves) or ISA-5.1 (instrumentation). Builders live in ``symbols_valves.py``, ``symbols_equipment.py`` and ``symbols_vessels.py`` and register here; this module holds the framework plus the instrument bubble, markers and connectors. """ from __future__ import annotations import hashlib import json import math from collections.abc import Callable from dataclasses import dataclass, field from .geometry import attdef, bbox_of, circle, dashed_line, line, polyline, regular_polygon, solid CATALOG_VERSION = "1" BUBBLE_RADIUS = 5.0 FAMILY_LAYER = { "valve": "PROCESS-VALVES", "rotating": "PROCESS-EQUIPMENT", "vessel": "PROCESS-EQUIPMENT", "heat": "PROCESS-EQUIPMENT", "misc": "PROCESS-EQUIPMENT", "instrument": "INSTRUMENT-SYMBOL", "connector": "PROCESS-EQUIPMENT", "marker": "line", } INSTRUMENT_TYPES = ("discrete", "dcs", "computer", "plc") INSTRUMENT_LOCATIONS = ("field", "primary", "auxiliary", "primary_rear", "auxiliary_rear") TAG_HEIGHT = 2.5 DESC_HEIGHT = 2.0 @dataclass(frozen=True) class Port: name: str x: float y: float direction_deg: float | None # outward direction a line leaves along; None = radial kind: str = "process" # "process" | "signal" radius: float = 0.0 # radial ports only def to_dict(self) -> dict: return { "name": self.name, "x": self.x, "y": self.y, "direction_deg": self.direction_deg, "kind": self.kind, "radius": self.radius, } @dataclass(frozen=True) class SymbolSpec: name: str family: str symbol: str variant: str | None primitives: tuple[dict, ...] attdefs: tuple[dict, ...] ports: tuple[Port, ...] bbox: tuple[float, float, float, float] layer_class: str source: str parametric: bool = False params: dict = field(default_factory=dict) def to_dict(self) -> dict: return { "name": self.name, "family": self.family, "symbol": self.symbol, "variant": self.variant, "ports": [p.to_dict() for p in self.ports], "bbox": list(self.bbox), "layer_class": self.layer_class, "source": self.source, "parametric": self.parametric, "params": dict(self.params), } Builder = Callable[..., SymbolSpec] _REGISTRY: dict[str, tuple[str, Builder, dict[str, list[str]], dict | None]] = {} def register( symbol: str, family: str, builder: Builder, variants: dict[str, list[str]] | None = None, params_schema: dict | None = None, ) -> None: if family not in FAMILY_LAYER: raise ValueError(f"unknown family {family!r}") _REGISTRY[symbol] = (family, builder, dict(variants or {}), params_schema) def resolve(symbol: str, **options) -> SymbolSpec: """Build the SymbolSpec for ``symbol`` with the given variant options.""" if symbol not in _REGISTRY: raise ValueError( f"unknown symbol {symbol!r}; valid symbols: {', '.join(sorted(_REGISTRY))}" ) family, builder, variants, schema = _REGISTRY[symbol] for key, value in options.items(): if value is None: continue if key == "params": if schema is None: raise ValueError(f"{symbol} is not parametric; 'params' is only for vessels") continue if key not in variants: raise ValueError(f"{symbol} takes no option {key!r}") if value not in variants[key]: raise ValueError( f"{symbol}: {key} must be one of {', '.join(variants[key])}, got {value!r}" ) return builder(**{k: v for k, v in options.items() if v is not None}) def list_symbols(family: str | None = None) -> list[dict]: rows = [] for symbol, (fam, builder, variants, schema) in sorted(_REGISTRY.items()): if family and fam != family: continue base = builder() rows.append( { "symbol": symbol, "family": fam, "variants": variants, "ports": [p.to_dict() for p in base.ports], "parametric": base.parametric, "params_schema": schema or {}, "source": base.source, "block_name": base.name, } ) return rows def all_specs() -> list[SymbolSpec]: """Every base symbol plus every enumerable variant (for tests and the contact sheet).""" specs: list[SymbolSpec] = [] for symbol, (_fam, builder, variants, _schema) in sorted(_REGISTRY.items()): if not variants: specs.append(builder()) continue keys = sorted(variants) combos: list[dict] = [{}] for key in keys: combos = [{**c, key: v} for c in combos for v in variants[key]] for combo in combos: try: specs.append(builder(**combo)) except ValueError: continue # a combination the builder refuses (e.g. an actuator on a check valve) return specs def transform_port(port: Port, x: float, y: float, rotation_deg: float, scale: float) -> dict: """Symbol-local port → WCS, for an INSERT at (x, y) rotated by ``rotation_deg`` and uniformly scaled.""" a = math.radians(rotation_deg) lx, ly = port.x * scale, port.y * scale wx = x + lx * math.cos(a) - ly * math.sin(a) wy = y + lx * math.sin(a) + ly * math.cos(a) direction = None if port.direction_deg is None else (port.direction_deg + rotation_deg) % 360.0 return { "name": port.name, "x": wx, "y": wy, "direction_deg": direction, "kind": port.kind, "radius": port.radius * scale, "inferred": False, } def params_hash(params: dict) -> str: return ( hashlib.sha256(json.dumps(params, sort_keys=True, separators=(",", ":")).encode()) .hexdigest()[:8] .upper() ) def tag_attdefs(ymax: float, with_desc: bool = True) -> tuple[dict, ...]: out = [attdef("TAG", 0.0, ymax + 3.5, TAG_HEIGHT)] if with_desc: out.append(attdef("DESC", 0.0, ymax + 6.5, DESC_HEIGHT)) return tuple(out) def make_spec( name: str, family: str, symbol: str, variant: str | None, primitives, attdefs, ports, source: str, bbox=None, parametric: bool = False, params: dict | None = None, ) -> SymbolSpec: prims = tuple(primitives) return SymbolSpec( name=name, family=family, symbol=symbol, variant=variant, primitives=prims, attdefs=tuple(attdefs), ports=tuple(ports), bbox=tuple(bbox) if bbox else bbox_of(prims), layer_class=FAMILY_LAYER[family], source=source, parametric=parametric, params=dict(params or {}), ) # ── instrument bubble (ISA-5.1-2009 Table 5.4.1) ───────────────────────────── def build_instrument(type: str = "discrete", location: str = "field") -> SymbolSpec: 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) 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)])] ys = { "field": [], "primary": [0.0], "auxiliary": [0.8, -0.8], "primary_rear": [0.0], "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)] attdefs = ( attdef("FUNC", 0.0, 2.0, DESC_HEIGHT, align="middle_center"), attdef("LOOP", 0.0, -2.0, DESC_HEIGHT, align="middle_center"), ) ports = (Port("signal", 0.0, 0.0, None, "signal", r),) return make_spec( f"PID_INST_{type.upper()}_{location.upper()}", "instrument", "instrument", f"{type}_{location}", prims, attdefs, ports, "ISA-5.1-2009 Table 5.4.1", bbox=(-r, -r, r, r), ) register( "instrument", "instrument", build_instrument, variants={"type": list(INSTRUMENT_TYPES), "location": list(INSTRUMENT_LOCATIONS)}, ) # ── markers (decoration on a line; never graph nodes) ──────────────────────── def build_marker(which: str) -> SymbolSpec: shapes = { "arrow_flow": [solid([(0, 0), (-3, 1.2), (-3, -1.2)])], "mark_pneumatic": [line(-1.5, -1.5, 0, 1.5), line(0.5, -1.5, 2, 1.5)], "mark_capillary": [line(-1.2, -1.2, 1.2, 1.2), line(-1.2, 1.2, 1.2, -1.2)], "mark_hydraulic": [line(-1, 1.5, -1, -1.5), line(-1, -1.5, 1.5, -1.5)], "mark_data": [circle(0, 0, 0.8)], } return make_spec( f"PID_MARKER_{which.upper()}", "marker", which, None, shapes[which], (), (), "ISA-5.1-2009 Table 5.3.1", ) for _which in ("arrow_flow", "mark_pneumatic", "mark_capillary", "mark_hydraulic", "mark_data"): register(_which, "marker", (lambda w: lambda: build_marker(w))(_which)) # ── connectors ─────────────────────────────────────────────────────────────── def build_connector(which: str = "offpage", direction: str = "out") -> SymbolSpec: if which == "offpage": if direction == "out": pts = [(-8, -3), (4, -3), (8, 0), (4, 3), (-8, 3)] port = Port("process", -8.0, 0.0, 180.0, "process") else: pts = [(8, -3), (-4, -3), (-8, 0), (-4, 3), (8, 3)] port = Port("process", 8.0, 0.0, 0.0, "process") attdefs = ( attdef( "TAG", -2.0 if direction == "out" else 2.0, 0.0, DESC_HEIGHT, align="middle_center" ), attdef("LINK", 0.0, -5.0, DESC_HEIGHT, invisible=True), ) return make_spec( f"PID_CONNECTOR_OFFPAGE_{direction.upper()}", "connector", "offpage", direction, [polyline(pts)], attdefs, (port,), "PIP PIC001 off-page connector", bbox=(-8, -3, 8, 3), ) if which == "tie_in": return make_spec( "PID_CONNECTOR_TIE_IN", "connector", "tie_in", None, [circle(0, 0, 3)], (attdef("TAG", 0.0, 0.0, DESC_HEIGHT, align="middle_center"),), (Port("in", -3.0, 0.0, 180.0), Port("out", 3.0, 0.0, 0.0)), "PIP PIC001 tie-in point", ) if which == "spec_break": return make_spec( "PID_CONNECTOR_SPEC_BREAK", "connector", "spec_break", None, [line(0, -3, 0, 3)], (attdef("TAG", 0.0, 4.5, DESC_HEIGHT),), (Port("in", -0.3, 0.0, 180.0), Port("out", 0.3, 0.0, 0.0)), "PIP PIC001 specification break", bbox=(-0.3, -3, 0.3, 4.5), ) raise ValueError(f"unknown connector {which!r}") register( "offpage", "connector", lambda direction="out": build_connector("offpage", direction), variants={"direction": ["in", "out"]}, ) register("tie_in", "connector", lambda: build_connector("tie_in")) register("spec_break", "connector", lambda: build_connector("spec_break")) # Builders for valves, equipment and vessels register themselves on import. from . import symbols_equipment as _symbols_equipment # noqa: E402,F401 from . import symbols_valves as _symbols_valves # noqa: E402,F401 from . import symbols_vessels as _symbols_vessels # noqa: E402,F401 ``` Until Tasks 8–10 exist, create the three builder modules as empty stubs so the import succeeds: ```python # engineering/pid/symbols_valves.py (replaced in Task 8) """Valve bodies and actuators — filled in by Task 8.""" ``` (same one-line docstring for `symbols_equipment.py` and `symbols_vessels.py`). - [ ] **Step 5: Run the tests** Run: `uv run --frozen pytest tests/test_pid_symbols.py tests/test_pid_xdata.py -v` Expected: all PASS (29 catalogue specs so far: 20 bubbles, 5 markers, 4 connectors). - [ ] **Step 6: Commit** ```bash git add engineering/pid/geometry.py engineering/pid/symbols.py engineering/pid/symbols_valves.py engineering/pid/symbols_equipment.py engineering/pid/symbols_vessels.py tests/test_pid_symbols.py git commit -m "feat(pid): symbol catalogue framework — instruments, markers, connectors" ``` --- ### Task 8: Valve bodies × actuators **Files:** - Replace: `engineering/pid/symbols_valves.py` - Test: `tests/test_pid_symbols.py` (append the valve-specific tests below) **Interfaces:** - Produces: `VALVE_BODIES = ("gate","globe","ball","butterfly","check","needle","plug","diaphragm","three_way","angle","relief")`, `ACTUATORS = ("none","diaphragm","piston","motor","solenoid","hand")`, `build_valve(body, actuator="none") -> SymbolSpec`; symbols registered as `resolve(body, actuator=...)` (one registry entry per body; variant key `actuator`). - [ ] **Step 1: Append the failing tests** ```python # append to tests/test_pid_symbols.py from engineering.pid.symbols_valves import ACTUATORS, VALVE_BODIES, build_valve def test_valve_bodies_and_actuators_enumerate(): assert len(VALVE_BODIES) == 11 and len(ACTUATORS) == 6 gate = build_valve("gate") assert gate.name == "PID_VALVE_GATE" and gate.bbox == (-4.0, -2.0, 4.0, 2.0) assert {p.name: (p.x, p.y, p.direction_deg) for p in gate.ports} == { "in": (-4.0, 0.0, 180.0), "out": (4.0, 0.0, 0.0), } assert [a["tag"] for a in gate.attdefs] == ["TAG", "DESC"] def test_actuators_add_a_signal_port_and_a_fail_attribute(): cv = build_valve("globe", "diaphragm") assert cv.name == "PID_VALVE_GLOBE_DIAPHRAGM" and cv.variant == "diaphragm" signal = next(p for p in cv.ports if p.name == "signal") assert (signal.x, signal.y, signal.direction_deg, signal.kind) == (0.0, 9.0, 90.0, "signal") assert cv.bbox[3] == 9.0 assert "FAIL" in [a["tag"] for a in cv.attdefs] hand = build_valve("gate", "hand") assert all(p.name != "signal" for p in hand.ports) and "FAIL" not in [ a["tag"] for a in hand.attdefs ] def test_check_and_relief_refuse_actuators(): for body in ("check", "relief"): with pytest.raises(ValueError, match="actuator"): build_valve(body, "motor") def test_three_way_angle_and_relief_ports(): assert {p.name for p in build_valve("three_way").ports} == {"in", "out", "branch"} angle = build_valve("angle") assert {p.name: p.direction_deg for p in angle.ports} == {"in": 180.0, "out": 270.0} relief = build_valve("relief") assert {p.name: p.direction_deg for p in relief.ports} == {"in": 270.0, "out": 0.0} ``` - [ ] **Step 2: Run to verify they fail** Run: `uv run --frozen pytest tests/test_pid_symbols.py -k "valve or actuator or relief" -v` Expected: FAIL (`ImportError: cannot import name 'ACTUATORS'`). - [ ] **Step 3: Write the module** Actuator placement is body-relative: the stem starts at the body's `axis_top` (where its own geometry on x = 0 ends) and the glyph base sits `STEM_CLEARANCE` above the body's top edge, so the needle body (top 5.6) is never overprinted and the diaphragm dome's base coincides with the stem top (no floating dome). ```python # engineering/pid/symbols_valves.py """Valve bodies (ISO 10628-2) composed with actuators (ISA-5.1 Table 5.5). Every body is authored once in symbol-local mm (reference size 8 × 4, process ports on the ``in`` / ``out`` faces) and any actuator stacks on top of it along the stem at x = 0: the stem starts where the body's own geometry on that axis ends (``axis_top``) and the actuator glyph sits ``STEM_CLEARANCE`` above the body's top edge, so a tall body (needle, diaphragm) pushes the actuator up instead of being overprinted by it. An actuated valve gains a ``signal`` port on its topmost edge and a ``FAIL`` attribute; check and relief valves take no actuator. """ from __future__ import annotations from .geometry import arc, attdef, bbox_of, circle, line, polyline, text from .symbols import DESC_HEIGHT, Port, SymbolSpec, make_spec, register, tag_attdefs VALVE_BODIES = ( "gate", "globe", "ball", "butterfly", "check", "needle", "plug", "diaphragm", "three_way", "angle", "relief", ) ACTUATORS = ("none", "diaphragm", "piston", "motor", "solenoid", "hand") NO_ACTUATOR = frozenset({"check", "relief"}) SOURCE = "ISO 10628-2:2012 valves; ISA-5.1-2009 Table 5.5 actuators" _LEFT = polyline([(-4, -2), (-4, 2), (0, 0)]) _RIGHT = polyline([(4, -2), (4, 2), (0, 0)]) _BOTTOM = polyline([(-2, -4), (2, -4), (0, 0)]) _IN = Port("in", -4.0, 0.0, 180.0) _OUT = Port("out", 4.0, 0.0, 0.0) _BOX = (-4.0, -2.0, 4.0, 2.0) # Gap between a body's top edge and the actuator glyph — the visible stem on the # reference 8 × 4 body (stem 0..4, glyph from y = 4). STEM_CLEARANCE = 2.0 # Extra stem (the yoke) between the clearance point and a diaphragm dome's base. DIAPHRAGM_YOKE = 2.0 # (primitives, process ports, declared bbox, axis_top) — ``axis_top`` is the y # where the body's own geometry on the x = 0 axis ends, i.e. where an actuator # stem may start without overprinting it. Body = tuple[list[dict], tuple[Port, ...], tuple[float, float, float, float], float] def _body(body: str) -> Body: """Primitives, process ports, declared bbox and axis top of a bare valve body.""" std = [_LEFT, _RIGHT] if body == "gate": return std, (_IN, _OUT), _BOX, 0.0 if body == "globe": return std + [circle(0, 0, 1.0)], (_IN, _OUT), _BOX, 1.0 if body == "ball": return std + [circle(0, 0, 1.8)], (_IN, _OUT), _BOX, 1.8 if body == "butterfly": return std + [line(0, -2, 0, 2), circle(0, 0, 0.8)], (_IN, _OUT), _BOX, 2.0 if body == "check": return std + [arc(0, 0, 2.5, 20, 160)], (_IN, _OUT), (-4.0, -2.0, 4.0, 2.5), 2.5 if body == "needle": prims = std + [line(0, 0, 0, 5), circle(0, 5, 0.6)] return prims, (_IN, _OUT), (-4.0, -2.0, 4.0, 5.6), 5.6 if body == "plug": return std + [polyline([(-1, -1), (1, -1), (1, 1), (-1, 1)])], (_IN, _OUT), _BOX, 1.0 if body == "diaphragm": return std + [arc(0, 2, 2, 0, 180)], (_IN, _OUT), (-4.0, -2.0, 4.0, 4.0), 4.0 if body == "three_way": ports = (_IN, _OUT, Port("branch", 0.0, -4.0, 270.0)) return std + [_BOTTOM], ports, (-4.0, -4.0, 4.0, 2.0), 0.0 if body == "angle": ports = (_IN, Port("out", 0.0, -4.0, 270.0)) return [_LEFT, _BOTTOM], ports, (-4.0, -4.0, 2.0, 2.0), 0.0 if body == "relief": spring = polyline( [(0, 0), (-1, 1.5), (1, 2.5), (-1, 3.5), (1, 4.5), (0, 6)], closed=False, ) ports = (Port("in", 0.0, -4.0, 270.0), _OUT) return [_BOTTOM, _RIGHT, spring], ports, (-2.0, -4.0, 4.0, 6.0), 6.0 raise ValueError(f"unknown valve body {body!r}; valid: {', '.join(VALVE_BODIES)}") def _actuator( actuator: str, body_top: float, axis_top: float = 0.0 ) -> tuple[list[dict], Port | None, float]: """Primitives above the body, the signal port (or None), and the new top y. The stem (always the first primitive) runs from ``axis_top`` — where the body's axial geometry ends — to the glyph base at ``body_top + STEM_CLEARANCE``; every glyph's lowest edge sits exactly on the stem's top, so an actuator is never drawn detached from, or over, its body. """ if actuator == "none": return [], None, body_top base = body_top + STEM_CLEARANCE if actuator == "hand": return [line(0, axis_top, 0, base), line(-2.5, base, 2.5, base)], None, base if actuator == "diaphragm": dome = base + DIAPHRAGM_YOKE prims = [line(0, axis_top, 0, dome), arc(0, dome, 3, 0, 180), line(-3, dome, 3, dome)] return prims, Port("signal", 0.0, dome + 3.0, 90.0, "signal"), dome + 3.0 stem = line(0, axis_top, 0, base) if actuator == "piston": top = base + 4.0 prims = [stem, polyline([(-2, base), (2, base), (2, top), (-2, top)])] return prims, Port("signal", 0.0, top, 90.0, "signal"), top if actuator == "motor": top = base + 4.0 prims = [stem, circle(0, base + 2.0, 2), text("M", 0, base + 2.0, 2.0)] return prims, Port("signal", 0.0, top, 90.0, "signal"), top if actuator == "solenoid": top = base + 3.0 box = polyline([(-1.5, base), (1.5, base), (1.5, top), (-1.5, top)]) prims = [stem, box, text("S", 0, base + 1.5, 1.8)] return prims, Port("signal", 0.0, top, 90.0, "signal"), top raise ValueError(f"unknown actuator {actuator!r}; valid: {', '.join(ACTUATORS)}") def build_valve(body: str = "gate", actuator: str = "none") -> SymbolSpec: """Compose ``body`` with ``actuator`` into a ``SymbolSpec``. Raises ``ValueError`` for an unknown body or actuator, and for an actuator on a check or relief valve (they are self-acting). """ prims, ports, box, axis_top = _body(body) if actuator not in ACTUATORS: raise ValueError(f"unknown actuator {actuator!r}; valid: {', '.join(ACTUATORS)}") if actuator != "none" and body in NO_ACTUATOR: raise ValueError(f"{body} valves take no actuator (got {actuator!r})") glyph_base = box[3] + STEM_CLEARANCE extra, signal, top = _actuator(actuator, box[3], axis_top) prims = list(prims) + extra ports = tuple(ports) + ((signal,) if signal else ()) if extra: # An actuator can be wider than a narrow body (angle: xmax 2 vs. a hand # bar at ±2.5 or a diaphragm at ±3), so union its real extent, not just y. ax0, ay0, ax1, ay1 = bbox_of(extra) box = (min(box[0], ax0), min(box[1], ay0), max(box[2], ax1), max(box[3], top, ay1)) attdefs = list(tag_attdefs(box[3])) if signal is not None: # Beside the actuator glyph (spec 4.7): right of its widest extent (±3 # for a dome), 1.5 above the glyph base. attdefs.append(attdef("FAIL", 3.5, glyph_base + 1.5, DESC_HEIGHT, align="left")) variant = None if actuator == "none" else actuator name = f"PID_VALVE_{body.upper()}" + (f"_{actuator.upper()}" if variant else "") return make_spec(name, "valve", body, variant, prims, attdefs, ports, SOURCE, bbox=box) def _valve_builder(body: str): def build(actuator: str = "none") -> SymbolSpec: return build_valve(body, actuator) return build for _body_name in VALVE_BODIES: register( _body_name, "valve", _valve_builder(_body_name), variants={"actuator": list(ACTUATORS)} ) ``` - [ ] **Step 4: Run the whole catalogue harness** Run: `uv run --frozen pytest tests/test_pid_symbols.py -v` Expected: all PASS — the harness now covers 11 bodies × 6 actuators minus the refused combinations (2 bodies × 5 actuators) = 56 valve specs plus the 29 from Task 7. - [ ] **Step 5: Commit** ```bash git add engineering/pid/symbols_valves.py tests/test_pid_symbols.py git commit -m "feat(pid): 11 ISO 10628-2 valve bodies composed with 6 ISA-5.1 actuators" ``` --- ### Task 9: Rotating equipment, heat transfer, miscellaneous inline symbols **Files:** - Replace: `engineering/pid/symbols_equipment.py` - Test: `tests/test_pid_symbols.py` (append) **Interfaces:** - Produces: `ROTATING = ("centrifugal_pump","pd_pump","compressor","fan","agitator","ejector")`, `HEAT = ("shell_tube_hx","plate_hx","air_cooler","fired_heater")`, `MISC = ("filter","strainer_y","reducer","flange_pair","spectacle_blind","orifice_plate","drain_vent")`, `build_equipment(symbol, **options)`; `reducer` has variant `shape ∈ {concentric, eccentric}`. - [ ] **Step 1: Append the failing tests** ```python # append to tests/test_pid_symbols.py from engineering.pid.symbols_equipment import HEAT, MISC, ROTATING def test_equipment_families_register(): assert len(ROTATING) == 6 and len(HEAT) == 4 and len(MISC) == 7 pump = resolve("centrifugal_pump") assert pump.family == "rotating" and pump.layer_class == "PROCESS-EQUIPMENT" assert {p.name: p.direction_deg for p in pump.ports} == {"suction": 180.0, "discharge": 90.0} hx = resolve("shell_tube_hx") assert {p.name for p in hx.ports} == {"shell_in", "shell_out", "tube_in", "tube_out"} orifice = resolve("orifice_plate") assert next(p for p in orifice.ports if p.name == "signal").kind == "signal" ecc = resolve("reducer", shape="eccentric") assert ecc.name == "PID_MISC_REDUCER_ECCENTRIC" assert next(p for p in ecc.ports if p.name == "out").y == pytest.approx(-1.0) assert resolve("reducer").name == "PID_MISC_REDUCER_CONCENTRIC" assert resolve("agitator").ports[0] == Port("shaft", 0.0, 0.0, 270.0, "process") ``` - [ ] **Step 2: Run to verify they fail** — `uv run --frozen pytest tests/test_pid_symbols.py -k equipment -v` → ImportError. - [ ] **Step 3: Write the module** ```python # engineering/pid/symbols_equipment.py """Rotating equipment, heat transfer and inline miscellany (ISO 10628-2).""" from __future__ import annotations from .geometry import circle, line, polyline, solid, text from .symbols import Port, SymbolSpec, make_spec, register, tag_attdefs ROTATING = ("centrifugal_pump", "pd_pump", "compressor", "fan", "agitator", "ejector") HEAT = ("shell_tube_hx", "plate_hx", "air_cooler", "fired_heater") MISC = ( "filter", "strainer_y", "reducer", "flange_pair", "spectacle_blind", "orifice_plate", "drain_vent", ) _SRC = "ISO 10628-2:2012" _IN, _OUT = Port("in", -4.0, 0.0, 180.0), Port("out", 4.0, 0.0, 0.0) def _rotating(symbol: str): if symbol == "centrifugal_pump": return ( [circle(0, 0, 4), line(0, 4, 0, 6), polyline([(-2, -1), (-2, 1), (1, 0)])], (Port("suction", -4.0, 0.0, 180.0), Port("discharge", 0.0, 6.0, 90.0)), (-4, -4, 4, 6), ) if symbol == "pd_pump": return ( [circle(0, 0, 4), polyline([(-2, -1.5), (2, -1.5), (0, 2)])], (_IN, _OUT), (-4, -4, 4, 4), ) if symbol == "compressor": return ([polyline([(-4, -3), (4, -1.5), (4, 1.5), (-4, 3)])], (_IN, _OUT), (-4, -3, 4, 3)) if symbol == "fan": return ( [circle(0, 0, 4), line(-2.8, -2.8, 2.8, 2.8), line(-2.8, 2.8, 2.8, -2.8)], (_IN, _OUT), (-4, -4, 4, 4), ) if symbol == "agitator": return ( [line(0, 0, 0, 8), circle(0, 10, 2), text("M", 0, 10, 2.0), line(-3, 0, 3, 0)], (Port("shaft", 0.0, 0.0, 270.0),), (-3, 0, 3, 12), ) if symbol == "ejector": return ( [ polyline([(-4, -1.5), (0, -1.5), (4, -0.75), (4, 0.75), (0, 1.5), (-4, 1.5)]), line(0, -1.5, 0, -4), ], (Port("motive", -4.0, 0.0, 180.0), Port("suction", 0.0, -4.0, 270.0), _OUT), (-4, -4, 4, 1.5), ) raise ValueError(symbol) def _heat(symbol: str): if symbol == "shell_tube_hx": return ( [ polyline([(-12, -4), (12, -4), (12, 4), (-12, 4)]), line(-9, -4, -9, 4), line(9, -4, 9, 4), ], ( Port("tube_in", -12.0, 0.0, 180.0), Port("tube_out", 12.0, 0.0, 0.0), Port("shell_in", -4.0, 4.0, 90.0), Port("shell_out", 4.0, -4.0, 270.0), ), (-12, -4, 12, 4), ) if symbol == "plate_hx": return ( [ polyline([(-4, -6), (4, -6), (4, 6), (-4, 6)]), line(-4, -6, 4, 6), line(-4, 6, 4, -6), ], ( Port("hot_in", -4.0, 3.0, 180.0), Port("hot_out", 4.0, -3.0, 0.0), Port("cold_in", 4.0, 3.0, 0.0), Port("cold_out", -4.0, -3.0, 180.0), ), (-4, -6, 4, 6), ) if symbol == "air_cooler": return ( [ polyline([(-10, -3), (10, -3), (10, 3), (-10, 3)]), circle(0, -6, 2.5), line(-1.8, -7.8, 1.8, -4.2), line(-1.8, -4.2, 1.8, -7.8), ], (Port("in", -10.0, 0.0, 180.0), Port("out", 10.0, 0.0, 0.0)), (-10, -8.5, 10, 3), ) if symbol == "fired_heater": return ( [ polyline([(-6, -8), (6, -8), (6, 4), (2, 4), (2, 10), (-2, 10), (-2, 4), (-6, 4)]), polyline([(-4, -6), (4, -4), (-4, -2), (4, 0), (-4, 2)], closed=False), ], ( Port("in", -6.0, -4.0, 180.0), Port("out", 6.0, 2.0, 0.0), Port("flue", 0.0, 10.0, 90.0), ), (-6, -8, 6, 10), ) raise ValueError(symbol) def _misc(symbol: str, shape: str = "concentric"): if symbol == "filter": return ( [polyline([(-4, -4), (4, -4), (4, 4), (-4, 4)]), line(-4, 4, 4, -4)], (_IN, _OUT), (-4, -4, 4, 4), ) if symbol == "strainer_y": return ( [line(-4, 0, 4, 0), line(0, 0, 3, -4), line(2, -4.7, 4, -3.3)], (_IN, _OUT), (-4, -5, 4, 0), ) if symbol == "reducer": if shape == "eccentric": return ( [polyline([(-3, -2), (3, -2), (3, 0), (-3, 2)])], (Port("in", -3.0, 0.0, 180.0), Port("out", 3.0, -1.0, 0.0)), (-3, -2, 3, 2), ) return ( [polyline([(-3, -2), (3, -1), (3, 1), (-3, 2)])], (Port("in", -3.0, 0.0, 180.0), Port("out", 3.0, 0.0, 0.0)), (-3, -2, 3, 2), ) if symbol == "flange_pair": return ( [line(-0.6, -2.5, -0.6, 2.5), line(0.6, -2.5, 0.6, 2.5)], (Port("in", -0.6, 0.0, 180.0), Port("out", 0.6, 0.0, 0.0)), (-0.6, -2.5, 0.6, 2.5), ) if symbol == "spectacle_blind": return ( [ line(0, -3.5, 0, 3.5), circle(0, 2, 1.5), solid([(0, -0.5), (1.5, -2), (0, -3.5), (-1.5, -2)]), ], (Port("in", -1.5, 0.0, 180.0), Port("out", 1.5, 0.0, 0.0)), (-1.5, -3.5, 1.5, 3.5), ) if symbol == "orifice_plate": return ( [line(-0.5, -3, -0.5, 3), line(0.5, -3, 0.5, 3)], ( Port("in", -0.5, 0.0, 180.0), Port("out", 0.5, 0.0, 0.0), Port("signal", 0.0, 3.0, 90.0, "signal"), ), (-0.5, -3, 0.5, 3), ) if symbol == "drain_vent": return ( [line(0, 0, 0, -4), line(-1.5, -4, 1.5, -4)], (Port("in", 0.0, 0.0, 90.0),), (-1.5, -4, 1.5, 0), ) raise ValueError(symbol) def build_equipment(symbol: str, shape: str = "concentric") -> SymbolSpec: if symbol in ROTATING: family, (prims, ports, box) = "rotating", _rotating(symbol) elif symbol in HEAT: family, (prims, ports, box) = "heat", _heat(symbol) elif symbol in MISC: family, (prims, ports, box) = "misc", _misc(symbol, shape) else: raise ValueError(f"unknown equipment {symbol!r}") variant = shape if symbol == "reducer" else None name = f"PID_{family.upper()}_{symbol.upper()}" + (f"_{variant.upper()}" if variant else "") return make_spec( name, family, symbol, variant, prims, tag_attdefs(box[3]), ports, _SRC, bbox=box ) for _name in ROTATING + HEAT + MISC: if _name == "reducer": register( _name, "misc", lambda shape="concentric": build_equipment("reducer", shape), variants={"shape": ["concentric", "eccentric"]}, ) else: _family = "rotating" if _name in ROTATING else ("heat" if _name in HEAT else "misc") register(_name, _family, (lambda s: lambda: build_equipment(s))(_name)) ``` - [ ] **Step 4: Run the harness** — `uv run --frozen pytest tests/test_pid_symbols.py -v` → all PASS (the `agitator` bbox `(-3, 0, 3, 12)` puts its shaft port on the bottom edge; `strainer_y`'s ports sit on its top edge `y = 0`). - [ ] **Step 5: Commit** ```bash git add engineering/pid/symbols_equipment.py tests/test_pid_symbols.py git commit -m "feat(pid): rotating, heat-transfer and inline equipment symbols" ``` --- ### Task 10: Parametric vessels **Files:** - Replace: `engineering/pid/symbols_vessels.py` - Test: `tests/test_pid_symbols.py` (append) **Interfaces:** - Produces: `VESSELS = ("vertical_vessel","horizontal_vessel","tank","column","reactor","hopper")`, `build_vessel(symbol, params=None) -> SymbolSpec` with `parametric=True`, `params` validated (`validate_vessel_params(symbol, params) -> dict` raising `ValueError` naming the key), block name `PID_VESSEL__<8 hex>`; default nozzles `N1 top 0.5, N2 bottom 0.5, N3 left 0.7, N4 right 0.3`. - Registry: `resolve("vertical_vessel", params={...})`; `list_symbols()` rows carry `params_schema`. - [ ] **Step 1: Append the failing tests** ```python # append to tests/test_pid_symbols.py from engineering.pid.symbols_vessels import VESSELS, build_vessel, validate_vessel_params def test_vessels_are_parametric_and_hash_named(): assert len(VESSELS) == 6 v = build_vessel("vertical_vessel") assert v.parametric is True and v.name.startswith("PID_VESSEL_VERTICAL_VESSEL_") assert len(v.name.rsplit("_", 1)[1]) == 8 assert {p.name: p.direction_deg for p in v.ports} == { "N1": 90.0, "N2": 270.0, "N3": 180.0, "N4": 0.0, } same = build_vessel("vertical_vessel", {"width": 20, "height": 40}) assert same.name == v.name, "identical parameters share one definition" other = build_vessel("vertical_vessel", {"width": 30}) assert other.name != v.name def test_vessel_nozzles_are_where_the_parameters_say(): v = build_vessel( "vertical_vessel", {"width": 20, "height": 40, "nozzles": [{"name": "F", "side": "right", "fraction": 0.25}]}, ) port = v.ports[0] assert (port.name, port.x, port.y, port.direction_deg) == ("F", 13.0, -10.0, 0.0) assert v.bbox == (-10.0, -20.0, 13.0, 20.0) def test_vessel_variants_draw_their_features(): column = build_vessel("column", {"trays": 5}) assert sum(1 for p in column.primitives if p["type"] == "line") >= 5 + 2 cone = build_vessel("tank", {"roof": "cone"}) assert any(p["type"] == "polyline" and not p["closed"] for p in cone.primitives) jacketed = build_vessel("reactor", {"jacketed": True}) assert jacketed.bbox[2] == pytest.approx(12.0 + 5.0), "half width 12 + jacket 2 + stub 3" hopper = build_vessel("hopper") assert {p.direction_deg for p in hopper.ports} == {90.0, 270.0} @pytest.mark.parametrize( "params, key", [ ({"width": 0}, "width"), ({"height": -1}, "height"), ({"nozzles": [{"name": "A", "side": "up", "fraction": 0.5}]}, "side"), ({"nozzles": [{"name": "A", "side": "top", "fraction": 1.5}]}, "fraction"), ( { "nozzles": [ {"name": "A", "side": "top", "fraction": 0.5}, {"name": "A", "side": "top", "fraction": 0.2}, ] }, "name", ), ({"trays": -2}, "trays"), ({"roof": "dome"}, "roof"), ({"bogus": 1}, "bogus"), ], ) def test_vessel_params_are_validated(params, key): symbol = "column" if "trays" in params else ("tank" if "roof" in params else "vertical_vessel") with pytest.raises(ValueError, match=key): validate_vessel_params(symbol, params) ``` - [ ] **Step 2: Run to verify they fail** — ImportError. - [ ] **Step 3: Write the module** ```python # engineering/pid/symbols_vessels.py """Parametric vessels (ISO 10628-2): size and nozzles come from the caller.""" from __future__ import annotations from .geometry import arc, line, polyline from .symbols import Port, SymbolSpec, make_spec, params_hash, register, tag_attdefs VESSELS = ("vertical_vessel", "horizontal_vessel", "tank", "column", "reactor", "hopper") SIDES = ("top", "bottom", "left", "right") DEFAULT_NOZZLES = [ {"name": "N1", "side": "top", "fraction": 0.5}, {"name": "N2", "side": "bottom", "fraction": 0.5}, {"name": "N3", "side": "left", "fraction": 0.7}, {"name": "N4", "side": "right", "fraction": 0.3}, ] _DEFAULTS = { "vertical_vessel": {"width": 20.0, "height": 40.0}, "horizontal_vessel": {"width": 40.0, "height": 20.0}, "tank": {"width": 24.0, "height": 24.0, "roof": "flat"}, "column": {"width": 16.0, "height": 60.0, "trays": 0}, "reactor": {"width": 24.0, "height": 36.0, "jacketed": False}, "hopper": {"width": 24.0, "height": 24.0}, } PARAMS_SCHEMA = { "width": "float > 0 (mm)", "height": "float > 0 (mm)", "nozzles": "[{name, side: top|bottom|left|right, fraction: 0..1}]", "roof": "tank only: flat | cone", "trays": "column only: int >= 0", "jacketed": "reactor only: bool", } STUB = 3.0 def validate_vessel_params(symbol: str, params: dict | None) -> dict: if symbol not in VESSELS: raise ValueError(f"unknown vessel {symbol!r}; valid: {', '.join(VESSELS)}") merged = dict(_DEFAULTS[symbol]) merged["nozzles"] = [dict(n) for n in DEFAULT_NOZZLES] if symbol == "hopper": merged["nozzles"] = [dict(n) for n in DEFAULT_NOZZLES[:2]] for key, value in (params or {}).items(): if key not in merged: raise ValueError( f"{symbol}: unknown parameter {key!r}; valid: {', '.join(sorted(merged))}" ) merged[key] = value for key in ("width", "height"): v = merged[key] if isinstance(v, bool) or not isinstance(v, (int, float)) or v <= 0: raise ValueError(f"{symbol}: {key} must be a number > 0") merged[key] = float(v) if "roof" in merged and merged["roof"] not in ("flat", "cone"): raise ValueError(f"{symbol}: roof must be 'flat' or 'cone'") if "trays" in merged and ( isinstance(merged["trays"], bool) or not isinstance(merged["trays"], int) or merged["trays"] < 0 ): raise ValueError(f"{symbol}: trays must be an int >= 0") if "jacketed" in merged and not isinstance(merged["jacketed"], bool): raise ValueError(f"{symbol}: jacketed must be a bool") nozzles = merged["nozzles"] if not isinstance(nozzles, list): raise ValueError(f"{symbol}: nozzles must be a list") names: set[str] = set() clean = [] for i, n in enumerate(nozzles): if not isinstance(n, dict) or not isinstance(n.get("name"), str) or not n["name"]: raise ValueError(f"{symbol}: nozzles[{i}].name must be a non-empty string") if n["name"] in names: raise ValueError(f"{symbol}: nozzles[{i}].name {n['name']!r} is repeated") names.add(n["name"]) if n.get("side") not in SIDES: raise ValueError(f"{symbol}: nozzles[{i}].side must be one of {', '.join(SIDES)}") f = n.get("fraction", 0.5) if isinstance(f, bool) or not isinstance(f, (int, float)) or not 0.0 <= f <= 1.0: raise ValueError(f"{symbol}: nozzles[{i}].fraction must be within 0..1") clean.append({"name": n["name"], "side": n["side"], "fraction": float(f)}) merged["nozzles"] = clean return merged def _nozzles(params: dict, half_w: float, half_h: float, extra: float = 0.0): """Nozzle stub primitives and ports on a box of ±half_w × ±half_h.""" prims, ports = [], [] stub = STUB + extra for n in params["nozzles"]: side, f = n["side"], n["fraction"] if side == "top": x, y0, y1, d = -half_w + f * 2 * half_w, half_h, half_h + stub, 90.0 prims.append(line(x, y0, x, y1)) ports.append(Port(n["name"], x, y1, d)) elif side == "bottom": x, y0, y1, d = -half_w + f * 2 * half_w, -half_h, -half_h - stub, 270.0 prims.append(line(x, y0, x, y1)) ports.append(Port(n["name"], x, y1, d)) elif side == "left": y, x0, x1, d = -half_h + f * 2 * half_h, -half_w, -half_w - stub, 180.0 prims.append(line(x0, y, x1, y)) ports.append(Port(n["name"], x1, y, d)) else: y, x0, x1, d = -half_h + f * 2 * half_h, half_w, half_w + stub, 0.0 prims.append(line(x0, y, x1, y)) ports.append(Port(n["name"], x1, y, d)) return prims, ports def _capsule_vertical(w: float, h: float) -> list[dict]: r = w / 2 body = h / 2 - r return [ line(-r, -body, -r, body), line(r, -body, r, body), arc(0, body, r, 0, 180), arc(0, -body, r, 180, 360), ] def _capsule_horizontal(w: float, h: float) -> list[dict]: r = h / 2 body = w / 2 - r return [ line(-body, -r, body, -r), line(-body, r, body, r), arc(body, 0, r, 270, 90), arc(-body, 0, r, 90, 270), ] def build_vessel(symbol: str = "vertical_vessel", params: dict | None = None) -> SymbolSpec: p = validate_vessel_params(symbol, params) w, h = p["width"], p["height"] half_w, half_h = w / 2, h / 2 extra = 0.0 if symbol == "vertical_vessel": prims = _capsule_vertical(w, h) elif symbol == "horizontal_vessel": prims = _capsule_horizontal(w, h) elif symbol == "tank": roof = w / 6 if p["roof"] == "cone" else 0.0 prims = [ polyline( [ (-half_w, -half_h), (half_w, -half_h), (half_w, half_h - roof), (-half_w, half_h - roof), ] ) ] if roof: prims.append( polyline( [(-half_w, half_h - roof), (0, half_h), (half_w, half_h - roof)], closed=False ) ) elif symbol == "column": prims = _capsule_vertical(w, h) trays = p["trays"] for i in range(trays): y = -half_h + (i + 1) * h / (trays + 1) prims.append(line(-half_w + 1, y, half_w - 1, y)) elif symbol == "reactor": prims = _capsule_vertical(w, h) if p["jacketed"]: body = half_h - half_w prims += [ line(-half_w - 2, -body, -half_w - 2, body), line(half_w + 2, -body, half_w + 2, body), ] extra = 2.0 else: # hopper prims = [ polyline([(-half_w, half_h), (half_w, half_h), (w / 6, -half_h), (-w / 6, -half_h)]) ] nozzle_prims, ports = _nozzles(p, half_w, half_h, extra) prims += nozzle_prims xs = [-half_w - extra, half_w + extra] + [pt.x for pt in ports] ys = [-half_h, half_h] + [pt.y for pt in ports] box = (min(xs), min(ys), max(xs), max(ys)) return make_spec( f"PID_VESSEL_{symbol.upper()}_{params_hash(p)}", "vessel", symbol, None, prims, tag_attdefs(box[3]), ports, "ISO 10628-2:2012 vessels", bbox=box, parametric=True, params=p, ) for _name in VESSELS: register( _name, "vessel", (lambda s: lambda params=None: build_vessel(s, params))(_name), params_schema=PARAMS_SCHEMA, ) ``` - [ ] **Step 4: Run the harness** — `uv run --frozen pytest tests/test_pid_symbols.py -v` → all PASS. - [ ] **Step 5: Commit** ```bash git add engineering/pid/symbols_vessels.py tests/test_pid_symbols.py git commit -m "feat(pid): six parametric vessel symbols with nozzle ports" ``` --- ### Task 11: Symbol insertion service + `pid_symbol_list` / `pid_symbol_insert` (SECTION 17 opens) **Files:** - Modify: `engineering/layers.py` (`PID_LAYERS` + `PROCESS-LINE-TEXT`) - Create: `engineering/pid/insert.py` - Modify: `server.py` (new `SECTION 17: P&ID (2 tools)` header before the RESOURCES block; the count grows in later tasks), `discovery/aliases.py`, `README.md` (`157 tools` → `159 tools`), regenerate inventory - Modify: `engineering/pid/__init__.py` (re-exports) - Test: `tests/test_pid_insert.py` **Interfaces:** - Consumes: `resolve`, `transform_port`, `FAMILY_LAYER` (Task 7), `symbol_payload`, `write_payload`, `marker_payload` (Task 5), `parse_tag`, `split_instrument_tag` (Task 6), `backend.block_define`, `backend.block_insert`, `backend.block_list`, `backend.layer_list`, `backend.layer_create`. - Produces (`engineering/pid/insert.py`): - `async ensure_layer(backend, layer_name) -> bool` (True when created; P&ID layers get their `PID_LAYERS` definition, anything else colour 7 / Continuous) - `async ensure_block(backend, spec) -> bool` (True when defined now) - `async insert_symbol(backend, spec, x, y, rotation=0.0, scale=1.0, attributes=None, layer=None, payload=None) -> dict` with keys `handle, block_name, defined, layers_created, ports, layer` - `async place_symbol(backend, symbol, x, y, rotation=0.0, scale=1.0, tag=None, desc=None, fail=None, link=None, layer=None, **options) -> dict` — the tool body; adds `tag`, `tag_warnings`, `backend`. - `FAIL_POSITIONS = ("", "FO", "FC", "FL")` - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_insert.py """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 and 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 and 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()} ``` - [ ] **Step 2: Run to verify they fail** — `uv run --frozen pytest tests/test_pid_insert.py -v` → ModuleNotFoundError. - [ ] **Step 3: Add the layer and write the service** In `engineering/layers.py`, insert into `PID_LAYERS` after the `INSTRUMENT-TAG-TEXT` entry: ```python (("PROCESS-LINE-TEXT", 7, "Continuous", 0.25, "line numbers"),) ``` ```python # engineering/pid/insert.py """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: 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 ``` Add to `engineering/pid/__init__.py`: ```python from .insert import ensure_block, ensure_layer, insert_symbol, place_symbol # noqa: F401 from .symbols import CATALOG_VERSION, Port, SymbolSpec, all_specs, list_symbols, resolve # noqa: F401 from .tags import parse_tag, split_instrument_tag # noqa: F401 ``` - [ ] **Step 4: Run the tests** — `uv run --frozen pytest tests/test_pid_insert.py tests/test_pid_symbols.py -v` → all PASS. (`backend.name` is the existing backend name property used by `system_status`; it returns `"ezdxf"` / `"com"`.) - [ ] **Step 5: Register the two tools** In `server.py`, before the `# ── RESOURCES` block, add: ```python # --------------------------------------------------------------------------- # ── 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, ) ``` Also insert `"pid"` at the **front** of `_GROUP_TAG_PRIORITY` and add `"pid": "pid"` to `_GROUP_TAG_LABELS`. Alias records: ```python "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"), ), ``` Regenerate the inventory; README `159 tools`. - [ ] **Step 6: Run the gates and commit** Run: `uv run --frozen pytest tests/test_pid_insert.py tests/test_discovery_aliases.py tests/test_tool_inventory.py tests/test_release_consistency.py tests/test_tool_registry.py -v` ```bash git add engineering/layers.py engineering/pid/insert.py engineering/pid/__init__.py server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_pid_insert.py git commit -m "feat(pid): pid_symbol_list and pid_symbol_insert — blocks with tags, ports and XDATA" ``` --- ### Task 12: Pure routing, labels, markers, crossings, line numbers (`engineering/pid/lines.py`) **Files:** - Create: `engineering/pid/lines.py` - Test: `tests/test_pid_lines.py` **Interfaces:** - Produces: `LineClass(layer, linetype, marker, arrow, kind)`, `LINE_CLASSES: dict[str, LineClass]`, `LAYER_TO_CLASS`, `PID_LINE_LAYERS`, `AXIS`, `snap_axis(dx, dy) -> float`, `route(start, start_dir, end, end_dir, stub=5.0, mode="auto") -> list[tuple[float, float]]`, `simplify(points)`, `path_length(points)`, `label_placement(vertices, offset=1.5) -> (x, y, rotation_deg, segment_index)`, `marker_positions(vertices, min_len=15.0) -> list[(x, y, rotation_deg)]`, `segments_cross(p1, p2, q1, q2) -> bool`, `count_crossings(vertices, others) -> int`, `format_line_number(fmt, **fields) -> str`, `DEFAULT_NUMBER_FORMAT`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_lines.py """Routing is geometry, so it is tested without a backend.""" from __future__ import annotations import pytest from engineering.pid.lines import ( DEFAULT_NUMBER_FORMAT, LINE_CLASSES, count_crossings, format_line_number, label_placement, marker_positions, route, snap_axis, ) def test_line_classes_map_to_layers_and_markers(): assert set(LINE_CLASSES) == { "process_major", "process_minor", "utility", "pneumatic", "electric", "hydraulic", "capillary", "data", } assert LINE_CLASSES["process_major"].layer == "PROCESS-PIPING-MAIN" assert ( LINE_CLASSES["electric"].layer == "INSTRUMENT-LINE-SIGNAL" and LINE_CLASSES["electric"].linetype is None ) assert ( LINE_CLASSES["pneumatic"].linetype == "Continuous" and LINE_CLASSES["pneumatic"].marker == "mark_pneumatic" ) assert LINE_CLASSES["process_major"].arrow is True and LINE_CLASSES["pneumatic"].arrow is False assert LINE_CLASSES["utility"].kind == "process" and LINE_CLASSES["data"].kind == "signal" def test_snap_axis_picks_the_dominant_component(): assert snap_axis(10, 1) == 0.0 and snap_axis(-10, 1) == 180.0 assert snap_axis(1, 10) == 90.0 and snap_axis(1, -10) == 270.0 def test_straight_when_aligned_and_facing(): assert route((0, 0), 0.0, (50, 0), 180.0) == [(0.0, 0.0), (50.0, 0.0)] def test_l_route_when_perpendicular(): path = route((0, 0), 0.0, (50, 30), 270.0) assert path == [(0.0, 0.0), (50.0, 0.0), (50.0, 30.0)] def test_z_route_when_parallel_and_offset(): path = route((0, 0), 0.0, (60, 20), 180.0, stub=5.0) assert path == [(0.0, 0.0), (30.0, 0.0), (30.0, 20.0), (60.0, 20.0)] def test_ports_facing_away_on_one_axis_need_waypoints(): with pytest.raises(ValueError, match="waypoints"): route((0, 0), 180.0, (40, 0), 0.0, stub=5.0) path = route((0, 0), 180.0, (40, 0), 0.0, mode=[(-5, 0), (-5, -10), (45, -10), (45, 0)]) assert len(path) == 6 def test_stub_is_honoured_or_refused(): with pytest.raises(ValueError, match="stub"): route((0, 0), 0.0, (3, 8), 180.0, stub=5.0) assert route((0, 0), 0.0, (3, 0), 180.0, stub=5.0) == [(0.0, 0.0), (3.0, 0.0)], ( "a straight run is exempt" ) def test_radial_ends_take_the_axis_towards_the_other_end(): path = route((0, 0), None, (0, 40), None) assert path == [(0.0, 0.0), (0.0, 40.0)] def test_direct_and_waypoints(): assert route((0, 0), 0.0, (10, 7), 90.0, mode="direct") == [(0.0, 0.0), (10.0, 7.0)] assert route((0, 0), 0.0, (10, 7), 90.0, mode=[(10, 0)]) == [ (0.0, 0.0), (10.0, 0.0), (10.0, 7.0), ] with pytest.raises(ValueError, match="repeat"): route((0, 0), 0.0, (10, 7), 90.0, mode=[(0, 0)]) with pytest.raises(ValueError, match="zero"): route((0, 0), 0.0, (0, 0), 90.0) def test_label_sits_left_of_the_longest_segment_and_never_upside_down(): x, y, rot, index = label_placement([(0, 0), (10, 0), (10, 40), (0, 40)]) assert index == 1 and rot == 90.0 assert (x, y) == pytest.approx((8.5, 20.0)) x, y, rot, index = label_placement([(50, 0), (0, 0)]) assert rot == 0.0 and y == pytest.approx(1.5) def test_markers_only_on_segments_long_enough(): marks = marker_positions([(0, 0), (10, 0), (10, 40)], min_len=15.0) assert marks == [(10.0, 20.0, 90.0)] assert marker_positions([(0, 0), (30, 0)]) == [(15.0, 0.0, 0.0)] def test_crossings_count_proper_intersections_only(): others = [[(5, -5), (5, 5)], [(20, 0), (20, 10)], [(0, 3), (8, 3)]] assert count_crossings([(0, 0), (10, 0)], others) == 1 assert count_crossings([(0, 0), (20, 0)], [[(20, 0), (20, 10)]]) == 0, ( "touching an endpoint is a junction" ) def test_line_number_format_drops_empty_fields(): assert DEFAULT_NUMBER_FORMAT == "{size}-{service}-{seq}-{spec}-{insulation}" assert ( format_line_number( DEFAULT_NUMBER_FORMAT, size="100", service="P", seq=1001, spec="CS1", insulation="IH" ) == "100-P-1001-CS1-IH" ) assert ( format_line_number(DEFAULT_NUMBER_FORMAT, size="100", service="P", seq=1001) == "100-P-1001" ) assert format_line_number("L-{seq}", seq=7) == "L-7" with pytest.raises(ValueError, match="unknown"): format_line_number("{bogus}", seq=1) ``` - [ ] **Step 2: Run to verify they fail** — ModuleNotFoundError. - [ ] **Step 3: Write the module** ```python # engineering/pid/lines.py """Line classes (ISA-5.1 Table 5.3.1), orthogonal routing, labels, markers, crossings.""" from __future__ import annotations import math import re from dataclasses import dataclass @dataclass(frozen=True) class LineClass: layer: str linetype: str | None # None = ByLayer marker: str | None # marker symbol placed on long segments arrow: bool # flow arrow at the end by default kind: str # "process" | "signal" LINE_CLASSES: dict[str, LineClass] = { "process_major": LineClass("PROCESS-PIPING-MAIN", None, None, True, "process"), "process_minor": LineClass("PROCESS-PIPING-SECONDARY", None, None, True, "process"), "utility": LineClass("UTILITY-LINE", None, None, True, "process"), "pneumatic": LineClass( "INSTRUMENT-LINE-SIGNAL", "Continuous", "mark_pneumatic", False, "signal" ), "electric": LineClass("INSTRUMENT-LINE-SIGNAL", None, None, False, "signal"), "hydraulic": LineClass( "INSTRUMENT-LINE-SIGNAL", "Continuous", "mark_hydraulic", False, "signal" ), "capillary": LineClass( "INSTRUMENT-LINE-SIGNAL", "Continuous", "mark_capillary", False, "signal" ), "data": LineClass("INSTRUMENT-LINE-SIGNAL", "Continuous", "mark_data", False, "signal"), } #: Layer → class when a line carries no payload. ``None`` = decide by linetype. LAYER_TO_CLASS: dict[str, str | None] = { "PROCESS-PIPING-MAIN": "process_major", "PROCESS-PIPING-SECONDARY": "process_minor", "UTILITY-LINE": "utility", "ELECTRICAL-LINE": "electric", "INSTRUMENT-LINE-SIGNAL": None, } PID_LINE_LAYERS = frozenset(LAYER_TO_CLASS) AXIS = {0.0: (1.0, 0.0), 90.0: (0.0, 1.0), 180.0: (-1.0, 0.0), 270.0: (0.0, -1.0)} DEFAULT_NUMBER_FORMAT = "{size}-{service}-{seq}-{spec}-{insulation}" _EPS = 1e-9 Point = tuple[float, float] def snap_axis(dx: float, dy: float) -> float: if abs(dx) >= abs(dy): return 0.0 if dx >= 0 else 180.0 return 90.0 if dy >= 0 else 270.0 def _unit(direction_deg: float) -> Point: return AXIS[float(direction_deg) % 360.0] def _dist(a: Point, b: Point) -> float: return math.hypot(b[0] - a[0], b[1] - a[1]) def path_length(points) -> float: return sum(_dist(a, b) for a, b in zip(points, points[1:])) def simplify(points) -> list[Point]: """Drop repeated points and merge collinear consecutive segments.""" pts: list[Point] = [] for p in points: q = (float(p[0]), float(p[1])) if not pts or _dist(pts[-1], q) > _EPS: pts.append(q) out: list[Point] = [] for p in pts: if len(out) >= 2: a, b = out[-2], out[-1] cross = (b[0] - a[0]) * (p[1] - b[1]) - (b[1] - a[1]) * (p[0] - b[0]) dot = (b[0] - a[0]) * (p[0] - b[0]) + (b[1] - a[1]) * (p[1] - b[1]) if abs(cross) < _EPS and dot > 0: out[-1] = p continue out.append(p) return out def _direction(a: Point, b: Point) -> Point: length = _dist(a, b) return ((b[0] - a[0]) / length, (b[1] - a[1]) / length) def _same(u: Point, v: Point) -> bool: return abs(u[0] - v[0]) < 1e-6 and abs(u[1] - v[1]) < 1e-6 def _validate(candidate, ds: float | None, de: float | None, stub: float) -> list[Point] | None: pts = simplify(candidate) if len(pts) < 2: return None segs = list(zip(pts, pts[1:])) straight = len(segs) == 1 if ds is not None and not _same(_direction(*segs[0]), _unit(ds)): return None if de is not None: ue = _unit(de) if not _same(_direction(*segs[-1]), (-ue[0], -ue[1])): return None minimum = 0.01 if straight else max(stub, 0.01) if any(_dist(a, b) < minimum - _EPS for a, b in segs): return None for (a1, b1), (a2, b2) in zip(segs, segs[1:]): u, v = _direction(a1, b1), _direction(a2, b2) if abs(u[0] * v[0] + u[1] * v[1]) > 1e-6: return None return pts def route(start, start_dir, end, end_dir, stub: float = 5.0, mode="auto") -> list[Point]: """Orthogonal path from ``start`` (leaving along ``start_dir``) to ``end`` (arriving against ``end_dir``). ``None`` directions are radial: the axis towards the other end. Raises ``ValueError`` when no candidate honours the stub.""" s: Point = (float(start[0]), float(start[1])) e: Point = (float(end[0]), float(end[1])) if isinstance(mode, (list, tuple)): path = [s] + [(float(p[0]), float(p[1])) for p in mode] + [e] for a, b in zip(path, path[1:]): if _dist(a, b) < 0.01: raise ValueError("waypoints repeat a point (segment shorter than 0.01 mm)") return path if _dist(s, e) < 0.01: raise ValueError("zero-length line: both ends are the same point") if mode == "direct": return [s, e] if mode != "auto": raise ValueError("route must be 'auto', 'direct' or a list of waypoints") if stub < 0: raise ValueError("stub must be >= 0") ds = snap_axis(e[0] - s[0], e[1] - s[1]) if start_dir is None else float(start_dir) % 360.0 de = snap_axis(s[0] - e[0], s[1] - e[1]) if end_dir is None else float(end_dir) % 360.0 us, ue = _unit(ds), _unit(de) s1 = (s[0] + us[0] * stub, s[1] + us[1] * stub) e1 = (e[0] + ue[0] * stub, e[1] + ue[1] * stub) xm, ym = (s1[0] + e1[0]) / 2.0, (s1[1] + e1[1]) / 2.0 candidates = [ [s, e], [s, (e[0], s[1]), e], [s, (s[0], e[1]), e], [s, (xm, s[1]), (xm, e[1]), e], [s, (s[0], ym), (e[0], ym), e], [s, s1, (s1[0], e1[1]), e1, e], [s, s1, (e1[0], s1[1]), e1, e], ] valid = [p for p in (_validate(c, ds, de, stub) for c in candidates) if p] if not valid: raise ValueError( f"no orthogonal route from {s} to {e} honours a {stub} mm stub on both ports; " "shorten `stub` or pass waypoints" ) valid.sort(key=lambda p: (len(p) - 2, path_length(p))) return valid[0] def label_placement(vertices, offset: float = 1.5) -> tuple[float, float, float, int]: """Midpoint of the longest segment, offset to its left, readable rotation.""" pts = [(float(x), float(y)) for x, y in vertices] segs = list(zip(pts, pts[1:])) index = max(range(len(segs)), key=lambda i: _dist(*segs[i])) (x0, y0), (x1, y1) = segs[index] angle = math.degrees(math.atan2(y1 - y0, x1 - x0)) % 360.0 if 90.0 < angle <= 270.0: angle = (angle - 180.0) % 360.0 ux, uy = math.cos(math.radians(angle)), math.sin(math.radians(angle)) mx, my = (x0 + x1) / 2.0, (y0 + y1) / 2.0 return (mx - uy * offset, my + ux * offset, round(angle, 6), index) def marker_positions(vertices, min_len: float = 15.0) -> list[tuple[float, float, float]]: pts = [(float(x), float(y)) for x, y in vertices] out = [] for (x0, y0), (x1, y1) in zip(pts, pts[1:]): if _dist((x0, y0), (x1, y1)) < min_len: continue angle = math.degrees(math.atan2(y1 - y0, x1 - x0)) % 360.0 out.append(((x0 + x1) / 2.0, (y0 + y1) / 2.0, round(angle, 6))) return out def _orient(a: Point, b: Point, c: Point) -> float: return (b[0] - a[0]) * (c[1] - a[1]) - (b[1] - a[1]) * (c[0] - a[0]) def segments_cross(p1, p2, q1, q2) -> bool: """Proper crossing only: shared endpoints and T-touches are junctions, not crossings.""" d1, d2 = _orient(q1, q2, p1), _orient(q1, q2, p2) d3, d4 = _orient(p1, p2, q1), _orient(p1, p2, q2) return d1 * d2 < -_EPS and d3 * d4 < -_EPS def count_crossings(vertices, others) -> int: pts = [(float(x), float(y)) for x, y in vertices] count = 0 for a, b in zip(pts, pts[1:]): for other in others: opts = [(float(x), float(y)) for x, y in other] for c, d in zip(opts, opts[1:]): if segments_cross(a, b, c, d): count += 1 return count _FIELD_RE = re.compile(r"^\{(\w+)\}$") _KNOWN_FIELDS = {"size", "service", "seq", "spec", "insulation", "area", "unit"} def format_line_number(fmt: str, **fields) -> str: """Fill ``fmt`` (``-``-separated tokens) and drop the empty fields with their separators.""" kept: list[str] = [] for token in fmt.split("-"): match = _FIELD_RE.match(token) if not match: kept.append(token) continue name = match.group(1) if name not in _KNOWN_FIELDS: raise ValueError( f"unknown line-number field {{{name}}}; known: {', '.join(sorted(_KNOWN_FIELDS))}" ) value = fields.get(name) if value is None or value == "": continue kept.append(str(value)) return "-".join(kept) ``` - [ ] **Step 4: Run the tests** — `uv run --frozen pytest tests/test_pid_lines.py -v` → all PASS. (Ports facing away along one axis have no orthogonal route: every U candidate collapses to a reversal after `simplify`, which the perpendicular check rejects — that is why the test expects the refusal and passes waypoints. The U candidates still serve ports facing away on *different* axes.) - [ ] **Step 5: Commit** ```bash git add engineering/pid/lines.py tests/test_pid_lines.py git commit -m "feat(pid): line classes, orthogonal routing, labels, markers, crossings" ``` --- ### Task 13: `pid_line_draw` **Files:** - Create: `engineering/pid/drawlines.py` - Modify: `server.py` (SECTION 17 header → `(3 tools)`), `discovery/aliases.py`, `README.md` (`160 tools`), inventory - Test: `tests/test_pid_line_draw.py` **Interfaces:** - Consumes: `route`, `label_placement`, `marker_positions`, `count_crossings`, `format_line_number`, `LINE_CLASSES`, `PID_LINE_LAYERS`, `snap_axis`, `AXIS` (Task 12); `insert_symbol`, `ensure_layer` (Task 11); `read_payload`, `write_payload`, `line_payload`, `marker_payload` (Task 5); `resolve`, `Port`, `transform_port` (Task 7); backend `entity_get`, `entity_list`, `entity_create_polyline`, `entity_set_properties`, `entity_create_text`. - Produces: `async resolve_endpoint(backend, ref) -> dict` (`x, y, direction_deg, kind, radius, handle, port, block_name`), `async existing_pid_lines(backend) -> list[dict]` (`handle, vertices, payload`), `async draw_line(backend, from_, to, line_class="process_major", route_mode="auto", stub=5.0, line_number=None, size=None, service=None, spec=None, insulation=None, number_format=None, label=True, arrow=None) -> dict` with keys `handle, vertices, line_class, layer, line_number, from, to, port_reuse, crossings, label_handle, arrow_handle, marker_handles, backend`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_line_draw.py """pid_line_draw: port-to-port orthogonal lines with class, number, markers, arrows.""" from __future__ import annotations import pytest from engineering.pid.drawlines import draw_line from engineering.pid.insert import place_symbol from engineering.pid.xdata import read_payload pytestmark = pytest.mark.asyncio async def _pump_and_vessel(backend): pump = await place_symbol(backend, "centrifugal_pump", 100, 100, tag="P-101") vessel = await place_symbol(backend, "vertical_vessel", 220, 120, tag="V-201") return pump, vessel async def test_process_line_between_ports(backend): pump, vessel = await _pump_and_vessel(backend) result = await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"handle": vessel["handle"], "port": "N3"}, line_class="process_major", size="100", service="P", spec="CS1", ) assert result["layer"] == "PROCESS-PIPING-MAIN" and result["line_class"] == "process_major" assert result["vertices"][0] == [100.0, 106.0] and result["vertices"][-1] == [207.0, 128.0] assert result["line_number"] == "100-P-1-CS1" and result["label_handle"] assert result["arrow_handle"] and result["marker_handles"] == [] assert result["crossings"] == 0 and result["port_reuse"] == [] info = await backend.entity_get(result["handle"]) assert info.type == "LWPOLYLINE" and info.layer == "PROCESS-PIPING-MAIN" payload = await read_payload(backend, result["handle"]) assert payload["kind"] == "line" and payload["from"] == { "handle": pump["handle"], "port": "discharge", } assert payload["seq"] == 1 label = await backend.entity_get(result["label_handle"]) assert label.layer == "PROCESS-LINE-TEXT" and label.properties["text"] == "100-P-1-CS1" arrow = await backend.entity_get(result["arrow_handle"]) assert ( arrow.properties["block_name"] == "PID_MARKER_ARROW_FLOW" and arrow.layer == "PROCESS-PIPING-MAIN" ) async def test_sequence_increments_and_number_can_be_given(backend): pump, vessel = await _pump_and_vessel(backend) first = await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"handle": vessel["handle"], "port": "N3"}, size="100", service="P", ) second = await draw_line( backend, {"handle": vessel["handle"], "port": "N4"}, {"x": 300, "y": 128}, size="80", service="P", ) assert (first["line_number"], second["line_number"]) == ("100-P-1", "80-P-2") third = await draw_line( backend, {"handle": vessel["handle"], "port": "N1"}, {"x": 210, "y": 200}, line_number="VENT-01", ) assert third["line_number"] == "VENT-01" async def test_signal_line_from_a_bubble_gets_markers_and_no_arrow(backend): cv = await place_symbol(backend, "globe", 100, 100, tag="FCV-1", actuator="diaphragm") fic = await place_symbol( backend, "instrument", 100, 160, tag="FIC-1", type="dcs", location="primary" ) result = await draw_line( backend, {"handle": fic["handle"]}, {"handle": cv["handle"], "port": "signal"}, line_class="pneumatic", ) assert result["vertices"][0] == [100.0, 155.0], "the line starts on the bubble's circle" assert result["vertices"][-1] == [100.0, 109.0] assert result["arrow_handle"] is None and len(result["marker_handles"]) == 1 mark = await backend.entity_get(result["marker_handles"][0]) assert mark.properties["block_name"] == "PID_MARKER_MARK_PNEUMATIC" assert mark.layer == "INSTRUMENT-LINE-SIGNAL" line = await backend.entity_get(result["handle"]) assert line.linetype == "Continuous" electric = await draw_line( backend, {"handle": fic["handle"]}, {"x": 160, "y": 160}, line_class="electric" ) assert (await backend.entity_get(electric["handle"])).linetype == "ByLayer" async def test_port_reuse_and_crossings_are_reported_not_refused(backend): pump, vessel = await _pump_and_vessel(backend) await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"handle": vessel["handle"], "port": "N3"}, ) again = await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"x": 100, "y": 200} ) assert again["port_reuse"] == [{"handle": pump["handle"], "port": "discharge"}] crossing = await draw_line( backend, {"x": 150, "y": 90}, {"x": 150, "y": 140}, line_class="utility" ) assert crossing["crossings"] == 1 async def test_refusals(backend): pump, vessel = await _pump_and_vessel(backend) with pytest.raises(ValueError, match="same"): await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"handle": pump["handle"], "port": "discharge"}, ) with pytest.raises(ValueError, match="discharge, suction"): # payload ports are JSON-sorted await draw_line(backend, {"handle": pump["handle"], "port": "outlet"}, {"x": 0, "y": 0}) with pytest.raises(ValueError, match="choose a port"): await draw_line(backend, {"handle": pump["handle"]}, {"x": 0, "y": 0}) with pytest.raises(ValueError, match="line_class"): await draw_line(backend, {"x": 0, "y": 0}, {"x": 10, "y": 0}, line_class="steam") first = await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"x": 100, "y": 200} ) with pytest.raises(ValueError, match="decoration"): await draw_line(backend, {"handle": first["arrow_handle"], "port": "in"}, {"x": 0, "y": 0}) with pytest.raises(ValueError, match="not a P&ID symbol"): line = await backend.entity_create_line(0, 0, 1, 1) await draw_line(backend, {"handle": line.handle, "port": "in"}, {"x": 0, "y": 0}) ``` - [ ] **Step 2: Run to verify they fail** — ModuleNotFoundError. - [ ] **Step 3: Write the orchestration** ```python # engineering/pid/drawlines.py """``pid_line_draw``: resolve ports, route, draw, label, mark, tag with XDATA.""" from __future__ import annotations from typing import TYPE_CHECKING from .insert import ensure_layer, insert_symbol from .lines import ( AXIS, DEFAULT_NUMBER_FORMAT, LINE_CLASSES, PID_LINE_LAYERS, count_crossings, format_line_number, label_placement, marker_positions, route, snap_axis, ) from .symbols import Port, resolve, transform_port from .xdata import line_payload, marker_payload, read_payload, write_payload if TYPE_CHECKING: from backends.base import AutoCADBackend TEXT_HEIGHT = 2.5 LABEL_LAYER = "PROCESS-LINE-TEXT" _INSERT_TYPES = {"INSERT", "BLOCKREFERENCE"} async def resolve_endpoint(backend: AutoCADBackend, ref: dict) -> dict: """``{handle, port}`` → the port in WCS (payload ports + INSERT transform); ``{x, y}`` → a free point.""" if not isinstance(ref, dict): raise ValueError("endpoint must be {handle, port} or {x, y}") if "handle" in ref: handle = str(ref["handle"]) info = await backend.entity_get(handle) block_name = str(info.properties.get("block_name", "")) if block_name.startswith("PID_MARKER_"): raise ValueError(f"{handle} is a marker/arrow — decoration on a line, not an endpoint") payload = await read_payload(backend, handle) if info.type not in _INSERT_TYPES or payload is None or payload.get("kind") != "symbol": raise ValueError(f"{handle} is not a P&ID symbol placed by pid_symbol_insert") ports = payload["ports"] port_name = ref.get("port") if port_name is None: if len(ports) == 1: port_name = next(iter(ports)) elif payload.get("family") == "instrument" and "signal" in ports: port_name = "signal" else: raise ValueError(f"{handle}: choose a port: {', '.join(ports)}") if port_name not in ports: raise ValueError(f"{handle} has no port {port_name!r}; ports: {', '.join(ports)}") lx, ly, ldir, kind, radius = ports[port_name] ins = info.properties["insertion"] world = transform_port( Port(port_name, float(lx), float(ly), ldir, kind, float(radius)), float(ins[0]), float(ins[1]), float(info.properties.get("rotation_deg", 0.0)), float(info.properties.get("x_scale", 1.0)), ) return {**world, "handle": handle, "port": port_name, "block_name": block_name} if "x" in ref and "y" in ref: return { "name": None, "x": float(ref["x"]), "y": float(ref["y"]), "direction_deg": None, "kind": None, "radius": 0.0, "inferred": False, "handle": None, "port": None, "block_name": None, } raise ValueError("endpoint must be {handle, port} or {x, y}") def _vertices(info) -> list[tuple[float, float]]: if info.type == "LINE": s, e = info.properties["start"], info.properties["end"] return [(float(s[0]), float(s[1])), (float(e[0]), float(e[1]))] return [(float(p[0]), float(p[1])) for p in info.properties.get("points", [])] async def existing_pid_lines(backend: AutoCADBackend) -> list[dict]: """Every LINE/LWPOLYLINE on a P&ID line layer, with its payload when it has one.""" out = [] for type_filter in ("LWPOLYLINE", "LINE"): offset = 0 while True: page = await backend.entity_list(type_filter=type_filter, limit=1000, offset=offset) for info in page: if info.layer.upper() not in PID_LINE_LAYERS: continue payload = ( await read_payload(backend, info.handle) if info.type == "LWPOLYLINE" else None ) out.append({"handle": info.handle, "vertices": _vertices(info), "payload": payload}) if len(page) < 1000: break offset += 1000 return out def _anchor(endpoint: dict, other: dict) -> tuple[tuple[float, float], float | None]: """Where the line actually starts/ends: on the circle for radial ports.""" if endpoint["radius"] > 0: axis = snap_axis(other["x"] - endpoint["x"], other["y"] - endpoint["y"]) ux, uy = AXIS[axis] return ( endpoint["x"] + ux * endpoint["radius"], endpoint["y"] + uy * endpoint["radius"], ), axis return (endpoint["x"], endpoint["y"]), endpoint["direction_deg"] def _ref(endpoint: dict) -> dict | None: return {"handle": endpoint["handle"], "port": endpoint["port"]} if endpoint["handle"] else None async def draw_line( backend: AutoCADBackend, from_: dict, to: dict, line_class: str = "process_major", route_mode="auto", stub: float = 5.0, line_number: str | None = None, size: str | None = None, service: str | None = None, spec: str | None = None, insulation: str | None = None, number_format: str | None = None, label: bool = True, arrow: bool | None = None, ) -> dict: if line_class not in LINE_CLASSES: raise ValueError(f"line_class must be one of {', '.join(LINE_CLASSES)}, got {line_class!r}") cls = LINE_CLASSES[line_class] start = await resolve_endpoint(backend, from_) end = await resolve_endpoint(backend, to) if start["handle"] and start["handle"] == end["handle"] and start["port"] == end["port"]: raise ValueError("from and to are the same port") s_point, s_dir = _anchor(start, end) e_point, e_dir = _anchor(end, start) path = route(s_point, s_dir, e_point, e_dir, stub=stub, mode=route_mode) existing = await existing_pid_lines(backend) crossings = count_crossings(path, [ln["vertices"] for ln in existing]) used: set[tuple[str, str]] = set() max_seq = 0 for ln in existing: payload = ln["payload"] or {} for key in ("from", "to"): ref = payload.get(key) if ref: used.add((ref["handle"], ref["port"])) if isinstance(payload.get("seq"), int): max_seq = max(max_seq, payload["seq"]) port_reuse = [ {"handle": ep["handle"], "port": ep["port"]} for ep in (start, end) if ep["handle"] and (ep["handle"], ep["port"]) in used ] seq = max_seq + 1 if line_number is None: fmt = number_format or DEFAULT_NUMBER_FORMAT fields = { "size": size, "service": service, "seq": seq, "spec": spec, "insulation": insulation, } line_number = format_line_number(fmt, **fields) or None await ensure_layer(backend, cls.layer) poly = await backend.entity_create_polyline( [list(p) for p in path], closed=False, layer=cls.layer ) if cls.linetype: await backend.entity_set_properties(poly.handle, linetype=cls.linetype) await write_payload( backend, poly.handle, line_payload( line_class, line_number, size, service, spec, insulation, seq, _ref(start), _ref(end) ), ) label_handle = None if label and line_number: await ensure_layer(backend, LABEL_LAYER) lx, ly, rot, _index = label_placement(path) import math shift = len(line_number) * TEXT_HEIGHT * 0.7 / 2.0 lx -= math.cos(math.radians(rot)) * shift ly -= math.sin(math.radians(rot)) * shift text = await backend.entity_create_text( line_number, lx, ly, TEXT_HEIGHT, rot, layer=LABEL_LAYER ) label_handle = text.handle want_arrow = cls.arrow if arrow is None else arrow arrow_handle = None if want_arrow: (ax0, ay0), (ax1, ay1) = path[-2], path[-1] angle = ( snap_axis(ax1 - ax0, ay1 - ay0) if abs(ax1 - ax0) < 1e-9 or abs(ay1 - ay0) < 1e-9 else math.degrees(math.atan2(ay1 - ay0, ax1 - ax0)) % 360.0 ) placed = await insert_symbol( backend, resolve("arrow_flow"), ax1, ay1, angle, 1.0, None, cls.layer, marker_payload(poly.handle), ) arrow_handle = placed["handle"] marker_handles: list[str] = [] if cls.marker: for mx, my, angle in marker_positions(path): placed = await insert_symbol( backend, resolve(cls.marker), mx, my, angle, 1.0, None, cls.layer, marker_payload(poly.handle), ) marker_handles.append(placed["handle"]) return { "handle": poly.handle, "vertices": [[x, y] for x, y in path], "line_class": line_class, "layer": cls.layer, "line_number": line_number, "from": { "handle": start["handle"], "port": start["port"], "x": s_point[0], "y": s_point[1], }, "to": {"handle": end["handle"], "port": end["port"], "x": e_point[0], "y": e_point[1]}, "port_reuse": port_reuse, "crossings": crossings, "label_handle": label_handle, "arrow_handle": arrow_handle, "marker_handles": marker_handles, "backend": backend.name, } ``` Move `import math` to the module's top-level imports (it is used in two places). - [ ] **Step 4: Run the tests** — `uv run --frozen pytest tests/test_pid_line_draw.py -v` → all PASS. The geometry behind the first test: the pump's `discharge` at rotation 0 is `(100, 106)` leaving along +Y; the vessel's `N3` (left side, fraction 0.7 of height 40, 3 mm stub) is `(220 - 10 - 3, 120 - 20 + 28) = (207, 128)` entered travelling +X; the one-bend route `[(100,106), (100,128), (207,128)]` satisfies both directions and the stub, so it is chosen. - [ ] **Step 5: Register the tool** `server.py` SECTION 17 header → `(3 tools)`; add after `pid_symbol_insert`: ```python @cad_tool( summary="Draw a P&ID line port-to-port: orthogonal route, ISA-5.1 class, line number, markers, arrow.", cost="mutate", ) @mcp.tool( annotations={"title": "P&ID: Draw Line", "readOnlyHint": False}, tags={"pid", "create"}, ) async def pid_line_draw( from_: Annotated[dict, "{handle, port} of a placed symbol, or {x, y}"], to: Annotated[dict, "{handle, port} of a placed symbol, or {x, y}"], line_class: Annotated[ str, "process_major | process_minor | utility | pneumatic | electric | hydraulic | capillary | data", ] = "process_major", route: Annotated[str | list, "auto (orthogonal) | direct | [[x, y], ...] waypoints"] = "auto", stub: Annotated[ float, Field(default=5.0, ge=0, description="Minimum straight run leaving each port (mm)") ] = 5.0, line_number: Annotated[ str | None, "Verbatim line number; omit to build one from the fields" ] = None, size: Annotated[str | None, "Nominal size, e.g. 100"] = None, service: Annotated[str | None, "Service code, e.g. P"] = None, spec: Annotated[str | None, "Piping spec, e.g. CS1"] = None, insulation: Annotated[str | None, "Insulation code, e.g. IH"] = None, number_format: Annotated[ str | None, "Default {size}-{service}-{seq}-{spec}-{insulation}; empty fields drop out" ] = None, label: Annotated[bool, "Write the line number along the longest segment"] = True, arrow: Annotated[ bool | None, "Flow arrow at the end; default per class (process yes, signal no)" ] = None, ctx: Context = None, ) -> dict: """Connect two ports with one LWPOLYLINE on the class's layer. Ports come from `pid_symbol_insert`; a bubble's radial port is entered without a name. `auto` picks the shortest orthogonal route that leaves each port straight for `stub` mm; when none exists the call refuses and says so — pass waypoints. `crossings` counts existing P&ID lines the new one cuts (reported, never refused) and `port_reuse` names ports that already had a line (branching is legal). Signal classes get ISA-5.1 markers (`//`, `X`, `L`, `o`) as small blocks on segments ≥ 15 mm; electric is dashed by layer. """ from engineering.pid.drawlines import draw_line await ctx.info(f"P&ID line {line_class}") return await draw_line( _backend(ctx), from_, to, line_class, route, stub, line_number, size, service, spec, insulation, number_format, label, arrow, ) ``` Alias record: ```python "pid_line_draw": ToolAliases( acad=("PLINE",), synonyms=("process line", "pipe run", "signal line", "connect equipment", "instrument line", "line number", "connect the pump to the vessel", "pneumatic signal"), ), ``` Regenerate inventory; README `160 tools`. - [ ] **Step 6: Gates and commit** Run: `uv run --frozen pytest tests/test_pid_line_draw.py tests/test_discovery_aliases.py tests/test_tool_inventory.py tests/test_release_consistency.py -v` ```bash git add engineering/pid/drawlines.py server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_pid_line_draw.py git commit -m "feat(pid): pid_line_draw — port-to-port orthogonal lines with class, number, markers" ``` --- ### Task 14: The reader — `engineering/pid/graph.py` + `pid_graph` **Files:** - Create: `engineering/pid/graph.py` - Modify: `server.py` (SECTION 17 → `(4 tools)`), `discovery/aliases.py`, `README.md` (`161 tools`), inventory - Test: `tests/test_pid_graph.py` **Interfaces:** - Consumes: `read_payload`, `Port`, `transform_port`, `LAYER_TO_CLASS`, `PID_LINE_LAYERS`, `parse_tag`, backend `entity_list`, `entity_get_xdata`, `block_get_attributes`, `layer_list`, `layout_list`, `layout_set_current`. - Produces: `async build_graph(backend, tolerance=0.5, label_search=15.0, scope="current_space", include_foreign=True, include_geometry=False) -> dict` with keys `nodes, edges, junctions, dangling, unclassified, offpage_links, stats, tolerance, scope, backend` (shapes in spec §9.1). Node `ports[name]` carries `edges: [edge handles]`. Edge `from`/`to` are `{"node": handle, "port": name}`, `{"junction": id}` or `None`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_graph.py """pid_graph reads the drawing back: our symbols exactly, foreign ones with confidence.""" from __future__ import annotations import pytest from engineering.pid.drawlines import draw_line from engineering.pid.graph import build_graph from engineering.pid.insert import place_symbol pytestmark = pytest.mark.asyncio async def _small_pid(backend): pump = await place_symbol(backend, "centrifugal_pump", 100, 100, tag="P-101", desc="Feed pump") hv = await place_symbol(backend, "gate", 160, 104, tag="HV-101", actuator="hand") vessel = await place_symbol(backend, "vertical_vessel", 220, 120, tag="V-201") fic = await place_symbol( backend, "instrument", 160, 160, tag="FIC-101", type="dcs", location="primary" ) l1 = await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"handle": hv["handle"], "port": "in"}, size="100", service="P", spec="CS1", ) l2 = await draw_line( backend, {"handle": hv["handle"], "port": "out"}, {"handle": vessel["handle"], "port": "N3"}, line_number=l1["line_number"], ) sig = await draw_line( backend, {"handle": fic["handle"]}, {"x": 160, "y": 112}, line_class="electric" ) return {"pump": pump, "hv": hv, "vessel": vessel, "fic": fic, "l1": l1, "l2": l2, "sig": sig} async def test_tool_drawn_pid_is_read_back_exactly(backend): ids = await _small_pid(backend) graph = await build_graph(backend) nodes = {n["id"]: n for n in graph["nodes"]} assert set(nodes) == { ids["pump"]["handle"], ids["hv"]["handle"], ids["vessel"]["handle"], ids["fic"]["handle"], } assert ( nodes[ids["pump"]["handle"]]["kind"] == "equipment" and nodes[ids["pump"]["handle"]]["tag"] == "P-101" ) assert nodes[ids["pump"]["handle"]]["description"] == "Feed pump" assert ( nodes[ids["fic"]["handle"]]["kind"] == "instrument" and nodes[ids["fic"]["handle"]]["tag"] == "FIC-101" ) assert nodes[ids["fic"]["handle"]]["tag_parsed"]["description"] == "Flow Indicating Controller" assert all(n["source"] == "catalog" and n["confidence"] == 1.0 for n in graph["nodes"]) edges = {e["id"]: e for e in graph["edges"]} assert set(edges) == {ids["l1"]["handle"], ids["l2"]["handle"], ids["sig"]["handle"]} l1 = edges[ids["l1"]["handle"]] assert l1["from"] == {"node": ids["pump"]["handle"], "port": "discharge"} assert l1["to"] == {"node": ids["hv"]["handle"], "port": "in"} assert ( l1["line_number"] == "100-P-1-CS1" and l1["number_source"] == "xdata" and l1["class_source"] == "xdata" ) assert nodes[ids["hv"]["handle"]]["ports"]["in"]["edges"] == [ids["l1"]["handle"]] sig = edges[ids["sig"]["handle"]] assert ( sig["from"] == {"node": ids["fic"]["handle"], "port": "signal"} and sig["line_class"] == "electric" ) assert len(graph["dangling"]) == 1 and graph["dangling"][0]["edge"] == ids["sig"]["handle"] assert graph["stats"]["nodes_by_kind"] == {"equipment": 2, "instrument": 1, "valve": 1} assert graph["stats"]["confidence_min"] == 1.0 and graph["stats"]["dangling"] == 1 assert graph["junctions"] == [] and graph["unclassified"] == [] async def test_a_nudged_symbol_makes_its_line_dangle_with_a_hint(backend): ids = await _small_pid(backend) await backend.entity_move(ids["hv"]["handle"], 0.0, 1.0) graph = await build_graph(backend, tolerance=0.5) dangling = {d["edge"]: d for d in graph["dangling"]} assert ids["l1"]["handle"] in dangling and ids["l2"]["handle"] in dangling hint = dangling[ids["l1"]["handle"]]["nearest"] assert ( hint["node"] == ids["hv"]["handle"] and hint["port"] == "in" and hint["distance"] == pytest.approx(1.0) ) assert await _no_dangling_after(backend, tolerance=1.5) async def _no_dangling_after(backend, tolerance): graph = await build_graph(backend, tolerance=tolerance) return graph["stats"]["dangling"] == 1 # only the deliberately open signal line async def test_branch_onto_a_line_becomes_a_junction(backend): ids = await _small_pid(backend) # l1 runs (100,106)->(100,111)->(151,111)->(151,104)->(156,104); (130,111) is on its second segment branch = await draw_line( backend, {"x": 130, "y": 111}, {"x": 130, "y": 60}, line_class="process_minor" ) graph = await build_graph(backend) edge = next(e for e in graph["edges"] if e["id"] == branch["handle"]) assert edge["from"] and "junction" in edge["from"] junction = next(j for j in graph["junctions"] if j["id"] == edge["from"]["junction"]) assert (junction["x"], junction["y"]) == pytest.approx((130.0, 111.0)) assert set(junction["edges"]) == {branch["handle"], ids["l1"]["handle"]} async def test_rotated_and_scaled_symbols_are_measured_not_assumed(backend): pump = await place_symbol( backend, "centrifugal_pump", 0, 0, rotation=90.0, scale=2.0, tag="P-1" ) graph = await build_graph(backend) node = graph["nodes"][0] assert node["ports"]["discharge"]["x"] == pytest.approx(-12.0) assert node["ports"]["discharge"]["direction_deg"] == pytest.approx(180.0) assert node["rotation_deg"] == pytest.approx(90.0) and node["scale"] == pytest.approx(2.0) async def test_foreign_blocks_classify_with_reduced_confidence_and_inferred_ports(backend): blk = backend._doc.blocks.new(name="GATE_VALVE") blk.add_lwpolyline([(-4, -2), (-4, 2), (0, 0)], close=True) blk.add_lwpolyline([(4, -2), (4, 2), (0, 0)], close=True) blk.add_attdef("TAG", insert=(0, 4), text="", dxfattribs={"height": 2.5}) logo = backend._doc.blocks.new(name="COMPANY_LOGO") logo.add_circle((0, 0), 5) await backend.block_insert("GATE_VALVE", 50, 50, attributes={"TAG": "HV-9"}) await backend.block_insert("COMPANY_LOGO", 300, 300) await backend.entity_create_line(10, 50, 46, 50, layer="PROCESS-PIPING-MAIN") graph = await build_graph(backend) assert len(graph["nodes"]) == 1 node = graph["nodes"][0] assert node["kind"] == "valve" and node["source"] == "heuristic" and node["confidence"] == 0.6 assert node["tag"] == "HV-9" and node["tag_source"] == "attribute" port = next(iter(node["ports"].values())) assert port["inferred"] is True and (port["x"], port["y"]) == pytest.approx((46.0, 50.0)) assert graph["stats"]["confidence_min"] == 0.6 edge = graph["edges"][0] assert edge["line_class"] == "process_major" and edge["class_source"] == "layer" assert graph["stats"]["entities_scanned"] >= 3 async def test_include_foreign_false_ignores_untagged_blocks(backend): blk = backend._doc.blocks.new(name="GATE_VALVE") blk.add_line((0, 0), (1, 0)) await backend.block_insert("GATE_VALVE", 50, 50) graph = await build_graph(backend, include_foreign=False) assert graph["nodes"] == [] and graph["edges"] == [] async def test_line_number_and_tags_from_nearby_text(backend): await backend.entity_create_line(0, 0, 100, 0, layer="PROCESS-PIPING-MAIN") await backend.entity_create_text('6"-P-1234-CS1', 40, 2, 2.5, layer="PROCESS-LINE-TEXT") graph = await build_graph(backend) edge = graph["edges"][0] assert edge["line_number"] == '6"-P-1234-CS1' and edge["number_source"] == "label" async def test_scope_all_walks_every_layout_and_restores_the_current_one(backend): await place_symbol(backend, "gate", 0, 0, tag="HV-1") await backend.layout_create("SHEET-2") await backend.layout_set_current("SHEET-2") await place_symbol(backend, "gate", 0, 0, tag="HV-2") await backend.layout_set_current("Model") graph = await build_graph(backend, scope="all") assert sorted(n["tag"] for n in graph["nodes"]) == ["HV-1", "HV-2"] assert sorted(n["space"] for n in graph["nodes"]) == ["Model", "SHEET-2"] assert (await backend.layout_list())["current"] == "Model" async def test_offpage_links_pair_by_link_id(backend): out = await place_symbol(backend, "offpage", 0, 0, tag="TO 002", link="L-17", direction="out") inn = await place_symbol( backend, "offpage", 100, 0, tag="FROM 001", link="L-17", direction="in" ) graph = await build_graph(backend) assert graph["offpage_links"] == [ {"link": "L-17", "nodes": sorted([out["handle"], inn["handle"]])} ] async def test_geometry_is_optional(backend): await _small_pid(backend) lean = await build_graph(backend) assert "vertices" not in lean["edges"][0] full = await build_graph(backend, include_geometry=True) assert len(full["edges"][0]["vertices"]) >= 2 ``` - [ ] **Step 2: Run to verify they fail** — ModuleNotFoundError. - [ ] **Step 3: Write the graph builder** ```python # engineering/pid/graph.py """Read a P&ID back as a graph: nodes (symbols), edges (lines), junctions, dangling ends. Rules the reader keeps (spec §9.4): it never modifies the drawing, it reads geometry from the drawing rather than the catalogue, and confidence is the minimum of its inputs — never raised by agreement between heuristics. """ from __future__ import annotations import math import re from dataclasses import dataclass, field from typing import TYPE_CHECKING from .lines import LAYER_TO_CLASS, PID_LINE_LAYERS from .symbols import Port, transform_port from .tags import parse_tag from .xdata import read_payload if TYPE_CHECKING: from backends.base import AutoCADBackend, EntityInfo INSERT_TYPES = {"INSERT", "BLOCKREFERENCE"} POLY_TYPES = {"LWPOLYLINE", "POLYLINE", "2DPOLYLINE"} LINE_TYPES = {"LINE"} TEXT_TYPES = {"TEXT", "MTEXT"} ATTRIBUTE_TAGS = ("TAG", "TAGNO", "TAGNUMBER", "EQUIP_TAG", "EQUIPMENT_TAG", "INST_TAG", "INSTTAG") KEYWORDS = { "VALVE": "valve", "VLV": "valve", "PUMP": "equipment", "TANK": "equipment", "VESSEL": "equipment", "DRUM": "equipment", "INST": "instrument", "BUBBLE": "instrument", "HX": "equipment", "EXCH": "equipment", "FILTER": "equipment", "COMP": "equipment", } FAMILY_KIND = {"instrument": "instrument", "valve": "valve", "connector": "connector"} LINE_NUMBER_RE = re.compile(r'^\d+(?:"|MM)?-[A-Z]{1,4}-\d+', re.IGNORECASE) CONFIDENCE = {"catalog": 1.0, "xdata": 0.95, "heuristic": 0.6, "inferred": 0.3} @dataclass class _Node: id: str kind: str block_name: str family: str | None symbol: str | None x: float y: float rotation_deg: float scale: float layer: str space: str bbox: tuple[float, float, float, float] | None source: str confidence: float attributes: dict ports: dict = field(default_factory=dict) tag: str | None = None tag_source: str | None = None def to_dict(self) -> dict: parsed = None if self.tag: parsed = parse_tag(self.tag, kind="instrument" if self.kind == "instrument" else "auto") return { "id": self.id, "kind": self.kind, "block_name": self.block_name, "family": self.family, "symbol": self.symbol, "tag": self.tag, "tag_parsed": parsed, "tag_source": self.tag_source, "x": self.x, "y": self.y, "rotation_deg": self.rotation_deg, "scale": self.scale, "layer": self.layer, "space": self.space, "ports": self.ports, "description": self.attributes.get("DESC") or None, "source": self.source, "confidence": self.confidence, } @dataclass class _Edge: id: str vertices: list[tuple[float, float]] layer: str linetype: str space: str payload: dict | None source: str line_class: str | None = None class_source: str | None = None line_number: str | None = None number_source: str | None = None from_ref: dict | None = None to_ref: dict | None = None def to_dict(self, include_geometry: bool) -> dict: p = self.payload or {} out = { "id": self.id, "line_class": self.line_class, "class_source": self.class_source, "line_number": self.line_number, "number_source": self.number_source, "size": p.get("size"), "service": p.get("service"), "spec": p.get("spec"), "insulation": p.get("insulation"), "from": self.from_ref, "to": self.to_ref, "length": round( sum(math.dist(a, b) for a, b in zip(self.vertices, self.vertices[1:])), 6 ), "layer": self.layer, "space": self.space, "source": self.source, } if include_geometry: out["vertices"] = [[x, y] for x, y in self.vertices] return out class _Grid: """Uniform grid for nearest-within-tolerance queries.""" def __init__(self, cell: float): self.cell = cell self.cells: dict[tuple[int, int], list] = {} def _key(self, x: float, y: float) -> tuple[int, int]: return (int(math.floor(x / self.cell)), int(math.floor(y / self.cell))) def add(self, x: float, y: float, item) -> None: self.cells.setdefault(self._key(x, y), []).append((x, y, item)) def near(self, x: float, y: float, radius: float): kx, ky = self._key(x, y) reach = int(math.ceil(radius / self.cell)) for dx in range(-reach, reach + 1): for dy in range(-reach, reach + 1): for px, py, item in self.cells.get((kx + dx, ky + dy), []): d = math.hypot(px - x, py - y) if d <= radius: yield d, item def _vertices(info: EntityInfo) -> list[tuple[float, float]]: if info.type in LINE_TYPES: s, e = info.properties["start"], info.properties["end"] return [(float(s[0]), float(s[1])), (float(e[0]), float(e[1]))] return [(float(p[0]), float(p[1])) for p in info.properties.get("points", [])] def _point_segment_distance(p, a, b) -> float: ax, ay = a bx, by = b px, py = p dx, dy = bx - ax, by - ay if dx == 0 and dy == 0: return math.hypot(px - ax, py - ay) t = max(0.0, min(1.0, ((px - ax) * dx + (py - ay) * dy) / (dx * dx + dy * dy))) return math.hypot(px - (ax + t * dx), py - (ay + t * dy)) def _bbox(info: EntityInfo): bb = info.properties.get("bounding_box") if not bb: return None return (float(bb["min"][0]), float(bb["min"][1]), float(bb["max"][0]), float(bb["max"][1])) def _on_bbox_boundary(p, bbox, tolerance) -> bool: x, y = p xmin, ymin, xmax, ymax = bbox inside_x = xmin - tolerance <= x <= xmax + tolerance inside_y = ymin - tolerance <= y <= ymax + tolerance near_vertical = min(abs(x - xmin), abs(x - xmax)) <= tolerance and inside_y near_horizontal = min(abs(y - ymin), abs(y - ymax)) <= tolerance and inside_x return near_vertical or near_horizontal async def _collect(backend: AutoCADBackend, scope: str) -> list[tuple[EntityInfo, str]]: async def page_all(space: str): out = [] offset = 0 while True: page = await backend.entity_list(limit=1000, offset=offset) out.extend((info, space) for info in page) if len(page) < 1000: return out offset += 1000 layouts = await backend.layout_list() current = layouts.get("current") or "Model" if scope != "all": return await page_all(current) entities = [] try: for name in layouts.get("layouts", [current]): await backend.layout_set_current(name) entities.extend(await page_all(name)) finally: await backend.layout_set_current(current) return entities async def build_graph( backend: AutoCADBackend, tolerance: float = 0.5, label_search: float = 15.0, scope: str = "current_space", include_foreign: bool = True, include_geometry: bool = False, ) -> dict: if tolerance <= 0: raise ValueError("tolerance must be > 0") if scope not in ("current_space", "all"): raise ValueError("scope must be 'current_space' or 'all'") entities = await _collect(backend, scope) layer_linetypes = {lyr.name.upper(): lyr.linetype.upper() for lyr in await backend.layer_list()} # 1 — classify INSERTs nodes: dict[str, _Node] = {} candidates: dict[str, _Node] = {} # unknown blocks kept only if a line touches them for info, space in entities: if info.type not in INSERT_TYPES: continue name = str(info.properties.get("block_name", "")) if name.startswith("PID_MARKER_"): continue payload = await read_payload(backend, info.handle) try: attributes = await backend.block_get_attributes(info.handle) except Exception: attributes = {} ins = info.properties.get("insertion", [0.0, 0.0]) rotation = float(info.properties.get("rotation_deg", 0.0)) scale = float(info.properties.get("x_scale", 1.0)) base = dict( id=info.handle, block_name=name, x=float(ins[0]), y=float(ins[1]), rotation_deg=rotation, scale=scale, layer=info.layer, space=space, bbox=_bbox(info), attributes=attributes, ) if payload and payload.get("kind") == "symbol": source = "catalog" if name.startswith("PID_") else "xdata" family = payload.get("family") node = _Node( kind=FAMILY_KIND.get(family, "equipment"), family=family, symbol=payload.get("symbol"), source=source, confidence=CONFIDENCE[source], **base, ) for pname, (lx, ly, ldir, kind, radius) in payload.get("ports", {}).items(): port = transform_port( Port(pname, float(lx), float(ly), ldir, kind, float(radius)), node.x, node.y, rotation, scale, ) node.ports[pname] = {**port, "edges": []} nodes[node.id] = node continue if not include_foreign: continue upper = name.upper() keyword_kind = next((k for word, k in KEYWORDS.items() if word in upper), None) tag_key = next((k for k in ATTRIBUTE_TAGS if k in attributes), None) if keyword_kind or tag_key: node = _Node( kind=keyword_kind or "equipment", family=None, symbol=None, source="heuristic", confidence=CONFIDENCE["heuristic"], **base, ) nodes[node.id] = node elif base["bbox"] is not None: candidates[info.handle] = _Node( kind="unknown_block", family=None, symbol=None, source="inferred", confidence=CONFIDENCE["inferred"], **base, ) # 2 — candidate edges edges: dict[str, _Edge] = {} foreign_edges: dict[str, _Edge] = {} for info, space in entities: if info.type not in POLY_TYPES | LINE_TYPES: continue verts = _vertices(info) if len(verts) < 2: continue payload = await read_payload(backend, info.handle) if info.type in POLY_TYPES else None layer = info.layer.upper() edge = _Edge( id=info.handle, vertices=verts, layer=info.layer, linetype=info.linetype, space=space, payload=payload if payload and payload.get("kind") == "line" else None, source="touch", ) if edge.payload: edge.source = "xdata" edge.line_class, edge.class_source = edge.payload.get("class"), "xdata" edge.line_number = edge.payload.get("number") edge.number_source = "xdata" if edge.line_number else None edges[edge.id] = edge elif layer in PID_LINE_LAYERS: edge.source = "layer" cls = LAYER_TO_CLASS[layer] if cls is None: effective = ( info.linetype.upper() if info.linetype.upper() != "BYLAYER" else layer_linetypes.get(layer, "") ) cls = "electric" if "DASH" in effective else "signal_unknown" edge.line_class, edge.class_source = cls, "layer" edges[edge.id] = edge elif include_foreign: foreign_edges[edge.id] = edge # 3 — spatial index of ports and vertices grid = _Grid(cell=10.0 * tolerance) radial_ports: list[tuple[str, str, float, float, float]] = [] for node in nodes.values(): for pname, port in node.ports.items(): if port.get("radius", 0.0) > 0: # a bubble's line starts on its circle, not at its centre radial_ports.append((node.id, pname, port["x"], port["y"], port["radius"])) else: grid.add(port["x"], port["y"], ("port", node.id, pname)) all_edges = {**edges, **foreign_edges} for edge in all_edges.values(): for v in edge.vertices: grid.add(v[0], v[1], ("vertex", edge.id)) # 4 — resolve endpoints junctions: list[dict] = [] dangling: list[dict] = [] def _junction_at(x, y, edge_id, other_id) -> dict: for j in junctions: if math.hypot(j["x"] - x, j["y"] - y) <= tolerance: for e in (edge_id, other_id): if e not in j["edges"]: j["edges"].append(e) return {"junction": j["id"]} j = {"id": f"J{len(junctions) + 1}", "x": x, "y": y, "edges": [edge_id, other_id]} junctions.append(j) return {"junction": j["id"]} def _resolve(edge: _Edge, end: str) -> dict | None: x, y = edge.vertices[0] if end == "from" else edge.vertices[-1] for node_id, pname, cx, cy, r in radial_ports: if abs(math.hypot(x - cx, y - cy) - r) <= tolerance: nodes[node_id].ports[pname]["edges"].append(edge.id) return {"node": node_id, "port": pname} best = None for d, item in grid.near(x, y, tolerance): if item[0] == "port" and (best is None or d < best[0]): best = (d, item) if best: _, (_kind, node_id, pname) = best nodes[node_id].ports[pname]["edges"].append(edge.id) return {"node": node_id, "port": pname} for node in list(nodes.values()) + list(candidates.values()): if ( node.source in ("heuristic", "inferred") and node.bbox and _on_bbox_boundary((x, y), node.bbox, tolerance) ): if node.id in candidates: nodes[node.id] = candidates.pop(node.id) pname = f"p{len(node.ports) + 1}" node.ports[pname] = { "name": pname, "x": x, "y": y, "direction_deg": None, "kind": "process", "radius": 0.0, "inferred": True, "edges": [edge.id], } grid.add(x, y, ("port", node.id, pname)) return {"node": node.id, "port": pname} for d, item in grid.near(x, y, tolerance): if item[0] == "vertex" and item[1] != edge.id: return _junction_at(x, y, edge.id, item[1]) for other in all_edges.values(): if other.id == edge.id: continue for a, b in zip(other.vertices, other.vertices[1:]): if _point_segment_distance((x, y), a, b) <= tolerance: return _junction_at(x, y, edge.id, other.id) nearest = None for d, item in grid.near(x, y, 10.0 * tolerance): if item[0] == "port" and (nearest is None or d < nearest[0]): nearest = (d, item) dangling.append( { "edge": edge.id, "end": end, "x": x, "y": y, "nearest": ( {"node": nearest[1][1], "port": nearest[1][2], "distance": round(nearest[0], 6)} if nearest else None ), } ) return None for edge in list(edges.values()): edge.from_ref = _resolve(edge, "from") edge.to_ref = _resolve(edge, "to") for edge in list(foreign_edges.values()): # a foreign line joins the graph only through a node probe_from = _resolve(edge, "from") probe_to = _resolve(edge, "to") attached = any(ref and "node" in ref for ref in (probe_from, probe_to)) if attached: edge.from_ref, edge.to_ref = probe_from, probe_to edges[edge.id] = edge else: dangling[:] = [d for d in dangling if d["edge"] != edge.id] for j in junctions: if edge.id in j["edges"]: j["edges"].remove(edge.id) junctions[:] = [j for j in junctions if len(j["edges"]) >= 2] # 5 — tags and line numbers from attributes or nearby text texts = [(info, space) for info, space in entities if info.type in TEXT_TYPES] def _text_near(x, y, radius, pattern=None): best = None for info, _space in texts: ins = info.properties.get("insertion") content = str(info.properties.get("text", "")).strip() if not ins or not content or (pattern and not pattern.match(content)): continue d = math.hypot(float(ins[0]) - x, float(ins[1]) - y) if d <= radius and (best is None or d < best[0]): best = (d, content) return best[1] if best else None for node in nodes.values(): attrs = node.attributes if node.kind == "instrument" and attrs.get("FUNC"): node.tag, node.tag_source = f"{attrs['FUNC']}-{attrs['LOOP']}".rstrip("-"), "attribute" else: key = next((k for k in ATTRIBUTE_TAGS if attrs.get(k)), None) if key: node.tag, node.tag_source = str(attrs[key]).strip(), "attribute" if node.tag is None: radius = max((p.get("radius", 0.0) for p in node.ports.values()), default=0.0) if node.kind == "instrument" and radius: found = _text_near(node.x, node.y, radius) else: cx = ((node.bbox[0] + node.bbox[2]) / 2.0) if node.bbox else node.x top = node.bbox[3] if node.bbox else node.y found = _text_near(cx, top, label_search) if found: node.tag, node.tag_source = found, "text" for edge in edges.values(): if edge.line_number: continue a, b = max(zip(edge.vertices, edge.vertices[1:]), key=lambda s: math.dist(*s)) found = _text_near((a[0] + b[0]) / 2.0, (a[1] + b[1]) / 2.0, label_search, LINE_NUMBER_RE) if found: edge.line_number, edge.number_source = found, "label" # 6 — off-page links links: dict[str, list[str]] = {} for node in nodes.values(): link = node.attributes.get("LINK") if node.kind == "connector" and link: links.setdefault(str(link), []).append(node.id) offpage_links = [{"link": k, "nodes": sorted(v)} for k, v in sorted(links.items())] # 7 — assemble node_rows = [n.to_dict() for n in nodes.values()] edge_rows = [e.to_dict(include_geometry) for e in edges.values()] by_kind: dict[str, int] = {} for n in node_rows: by_kind[n["kind"]] = by_kind.get(n["kind"], 0) + 1 by_class: dict[str, int] = {} for e in edge_rows: key = e["line_class"] or "unknown" by_class[key] = by_class.get(key, 0) + 1 return { "nodes": node_rows, "edges": edge_rows, "junctions": junctions, "dangling": dangling, "unclassified": [{"handle": c.id, "block_name": c.block_name} for c in candidates.values()], "offpage_links": offpage_links, "stats": { "nodes_by_kind": dict(sorted(by_kind.items())), "edges_by_class": dict(sorted(by_class.items())), "dangling": len(dangling), "unclassified": len(candidates), "confidence_min": min((n["confidence"] for n in node_rows), default=1.0), "entities_scanned": len(entities), }, "tolerance": tolerance, "scope": scope, "backend": backend.name, } ``` - [ ] **Step 4: Run the tests** — `uv run --frozen pytest tests/test_pid_graph.py -v` → PASS. Note for the foreign test: the INSERT of `GATE_VALVE` at (50, 50) has a bounding box of x ∈ [46, 54]; the line ends at (46, 50), on its left edge, so `_on_bbox_boundary` accepts it and creates `p1`. - [ ] **Step 5: Register `pid_graph`** SECTION 17 header → `(4 tools)`; add: ```python @cad_tool( summary="Read the P&ID back as a graph: symbols, lines, junctions, dangling ends, confidence.", cost="read", ) @mcp.tool( annotations={"title": "P&ID: Connectivity Graph", "readOnlyHint": True}, tags={"pid", "query"}, ) async def pid_graph( tolerance: Annotated[ float, Field(default=0.5, gt=0, description="Endpoint-to-port snap distance (mm)") ] = 0.5, label_search: Annotated[ float, Field(default=15.0, gt=0, description="How far to look for tag/line-number text (mm)"), ] = 15.0, scope: Annotated[ str, "current_space | all (every layout, current restored afterwards)" ] = "current_space", include_foreign: Annotated[ bool, "Classify blocks not placed by this server (confidence < 1)" ] = True, include_geometry: Annotated[bool, "Include edge vertices"] = False, ctx: Context = None, ) -> dict: """Connectivity of the drawing as it *is*, not as it was drawn. Nodes are block references classified by catalogue name (confidence 1.0), ACADMCP_PID payload (0.95), tag attributes / block-name keywords (0.6) or a line touching an unknown block (0.3); every node says which. Ports are measured from the INSERT's real rotation and scale. Edges are lines and polylines; each end resolves to a port, a junction on another line, or `dangling` with the nearest port as a hint. `stats.confidence_min` is the number to read before trusting a foreign drawing. """ from engineering.pid.graph import build_graph return await build_graph( _backend(ctx), tolerance, label_search, scope, include_foreign, include_geometry ) ``` Alias: ```python "pid_graph": ToolAliases( acad=(), synonyms=("p&id connectivity", "what is connected to", "trace the line", "read the p&id", "dangling lines", "instrument loop graph", "process graph"), ), ``` Inventory; README `161 tools`. - [ ] **Step 6: Gates and commit** ```bash git add engineering/pid/graph.py server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_pid_graph.py git commit -m "feat(pid): pid_graph reads the drawing back with ports, junctions, dangling ends and confidence" ``` --- ### Task 15: Deliverables — instrument index, line list, equipment list **Files:** - Create: `engineering/pid/deliverables.py` - Modify: `server.py` (SECTION 17 → `(7 tools)`), `discovery/aliases.py`, `README.md` (`164 tools`), inventory - Test: `tests/test_pid_deliverables.py` **Interfaces:** - Consumes: `build_graph` (Task 14), `validate_path` (`security.py`). - Produces: `instrument_index(graph) -> list[dict]`, `line_list(graph) -> list[dict]`, `equipment_list(graph) -> list[dict]`, `natural_key(tag)`, `write_csv(rows, path) -> str`, `async deliverable(backend, which, csv_path=None, **graph_kwargs) -> dict` (`rows, count, csv_path, graph_stats`). Row keys exactly as spec §9.5. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_deliverables.py """Instrument index, line list and equipment list are derived from the graph.""" from __future__ import annotations import csv import pytest from engineering.pid.deliverables import ( deliverable, equipment_list, instrument_index, line_list, natural_key, ) from engineering.pid.drawlines import draw_line from engineering.pid.graph import build_graph from engineering.pid.insert import place_symbol pytestmark = pytest.mark.asyncio INDEX_KEYS = [ "tag", "func", "loop", "description", "type", "location", "connected_to", "signal_lines", "handle", "confidence", ] LINE_KEYS = [ "line_number", "line_class", "size", "service", "spec", "insulation", "from_tag", "from_port", "to_tag", "to_port", "length_mm", "handle", "number_source", ] EQUIP_KEYS = [ "tag", "kind", "symbol", "description", "ports", "connected_lines", "unconnected_ports", "handle", "confidence", ] async def _pid(backend): pump = await place_symbol(backend, "centrifugal_pump", 100, 100, tag="P-101", desc="Feed pump") vessel = await place_symbol(backend, "vertical_vessel", 220, 120, tag="V-201") cv = await place_symbol(backend, "globe", 160, 106, tag="FCV-101", actuator="diaphragm") fic = await place_symbol( backend, "instrument", 160, 160, tag="FIC-101", type="dcs", location="primary" ) await place_symbol( backend, "instrument", 100, 160, tag="PT-10", type="discrete", location="field" ) await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"handle": cv["handle"], "port": "in"}, size="100", service="P", spec="CS1", ) await draw_line( backend, {"handle": cv["handle"], "port": "out"}, {"handle": vessel["handle"], "port": "N3"}, size="100", service="P", spec="CS1", ) await draw_line( backend, {"handle": fic["handle"]}, {"handle": cv["handle"], "port": "signal"}, line_class="pneumatic", ) return await build_graph(backend) async def test_instrument_index_rows(backend): graph = await _pid(backend) rows = instrument_index(graph) assert [list(r) for r in rows] == [INDEX_KEYS] * 2 assert [r["tag"] for r in rows] == ["FIC-101", "PT-10"] fic = rows[0] assert ( fic["description"] == "Flow Indicating Controller" and fic["type"] == "dcs" and fic["location"] == "primary" ) assert fic["connected_to"] == "FCV-101" and fic["signal_lines"] == 1 assert rows[1]["signal_lines"] == 0 and rows[1]["connected_to"] == "" async def test_line_list_rows(backend): graph = await _pid(backend) rows = line_list(graph) assert [list(r) for r in rows] == [LINE_KEYS] * 3 numbered = [r for r in rows if r["line_number"]] assert [r["line_number"] for r in numbered] == ["100-P-1-CS1", "100-P-2-CS1"] assert numbered[0]["from_tag"] == "P-101" and numbered[0]["from_port"] == "discharge" assert numbered[0]["to_tag"] == "FCV-101" and numbered[0]["length_mm"] > 0 assert rows[-1]["line_number"] is None, "untagged rows sort last" async def test_equipment_list_rows(backend): graph = await _pid(backend) rows = equipment_list(graph) assert [list(r) for r in rows] == [EQUIP_KEYS] * 3 assert [r["tag"] for r in rows] == ["FCV-101", "P-101", "V-201"] pump = next(r for r in rows if r["tag"] == "P-101") assert ( pump["description"] == "Feed pump" and pump["ports"] == 2 and pump["connected_lines"] == 1 ) assert pump["unconnected_ports"] == "suction" def test_natural_sort(): assert sorted(["P-10", "P-2", "P-1A"], key=natural_key) == ["P-1A", "P-2", "P-10"] assert natural_key(None) > natural_key("Z-999"), "untagged rows sort last" async def test_csv_is_written_inside_allowed_paths(backend, tmp_path, monkeypatch): import config monkeypatch.setattr(config.settings, "allowed_paths", [tmp_path.resolve()]) await _pid(backend) target = tmp_path / "index.csv" result = await deliverable(backend, "instrument_index", csv_path=str(target)) assert result["count"] == 2 and result["csv_path"] == str(target.resolve()) with target.open(newline="", encoding="utf-8") as fh: reader = list(csv.reader(fh)) assert reader[0] == INDEX_KEYS and reader[1][0] == "FIC-101" with pytest.raises(Exception): await deliverable(backend, "line_list", csv_path=str(tmp_path.parent / "outside.csv")) ``` - [ ] **Step 2: Run to verify they fail** — ModuleNotFoundError. - [ ] **Step 3: Write the module** ```python # engineering/pid/deliverables.py """Instrument index, line list and equipment list — rows derived from the graph.""" from __future__ import annotations import csv import re from pathlib import Path from typing import TYPE_CHECKING from .graph import build_graph if TYPE_CHECKING: from backends.base import AutoCADBackend _NUM = re.compile(r"(\d+)") def natural_key(tag): if tag is None: return (1, []) return (0, [int(part) if part.isdigit() else part for part in _NUM.split(str(tag))]) def _node_map(graph: dict) -> dict[str, dict]: return {n["id"]: n for n in graph["nodes"]} def _tag_of(ref: dict | None, nodes: dict) -> tuple[str | None, str | None]: if not ref: return None, None if "node" in ref: return nodes[ref["node"]].get("tag"), ref.get("port") return ref.get("junction"), None def instrument_index(graph: dict) -> list[dict]: nodes = _node_map(graph) edges = {e["id"]: e for e in graph["edges"]} rows = [] for node in graph["nodes"]: if node["kind"] != "instrument": continue parsed = node.get("tag_parsed") or {} connected = [] signal_lines = 0 for port in node["ports"].values(): for edge_id in port["edges"]: edge = edges[edge_id] signal_lines += 1 for ref in (edge["from"], edge["to"]): if ref and "node" in ref and ref["node"] != node["id"]: other = nodes[ref["node"]] connected.append( other.get("tag") or (edge.get("line_number") or other["id"]) ) variant = (node.get("symbol") or "", node.get("block_name") or "") rows.append( { "tag": node.get("tag"), "func": parsed.get("letters"), "loop": parsed.get("loop"), "description": parsed.get("description"), "type": _bubble_part(node, 2), "location": _bubble_part(node, 3), "connected_to": ", ".join(dict.fromkeys(connected)), "signal_lines": signal_lines, "handle": node["id"], "confidence": node["confidence"], } ) rows.sort(key=lambda r: natural_key(r["tag"])) return rows def _bubble_part(node: dict, index: int) -> str | None: """PID_INST__ → type (index 2) or location (index 3+).""" name = node.get("block_name") or "" if not name.startswith("PID_INST_"): return None parts = name.split("_") if index == 2: return parts[2].lower() if len(parts) > 2 else None return "_".join(parts[3:]).lower() if len(parts) > 3 else None def line_list(graph: dict) -> list[dict]: nodes = _node_map(graph) rows = [] for edge in graph["edges"]: from_tag, from_port = _tag_of(edge["from"], nodes) to_tag, to_port = _tag_of(edge["to"], nodes) rows.append( { "line_number": edge.get("line_number"), "line_class": edge.get("line_class"), "size": edge.get("size"), "service": edge.get("service"), "spec": edge.get("spec"), "insulation": edge.get("insulation"), "from_tag": from_tag, "from_port": from_port, "to_tag": to_tag, "to_port": to_port, "length_mm": round(float(edge.get("length") or 0.0), 1), "handle": edge["id"], "number_source": edge.get("number_source"), } ) rows.sort(key=lambda r: natural_key(r["line_number"])) return rows def equipment_list(graph: dict) -> list[dict]: rows = [] for node in graph["nodes"]: if node["kind"] not in ("equipment", "valve", "connector", "unknown_block"): continue ports = node["ports"] connected = sum(1 for p in ports.values() if p["edges"]) rows.append( { "tag": node.get("tag"), "kind": node["kind"], "symbol": node.get("symbol"), "description": node.get("description") or (node.get("tag_parsed") or {}).get("equipment_kind"), "ports": len(ports), "connected_lines": sum(len(p["edges"]) for p in ports.values()), "unconnected_ports": ", ".join(name for name, p in ports.items() if not p["edges"]), "handle": node["id"], "confidence": node["confidence"], } ) rows.sort(key=lambda r: natural_key(r["tag"])) return rows def write_csv(rows: list[dict], path: str) -> str: from security import validate_path target = validate_path(path, allow_write=True) Path(target).parent.mkdir(parents=True, exist_ok=True) with open(target, "w", newline="", encoding="utf-8") as fh: writer = csv.writer(fh) if rows: writer.writerow(list(rows[0])) for row in rows: writer.writerow(["" if v is None else v for v in row.values()]) return str(target) _BUILDERS = { "instrument_index": instrument_index, "line_list": line_list, "equipment_list": equipment_list, } async def deliverable( backend: AutoCADBackend, which: str, csv_path: str | None = None, **graph_kwargs ) -> dict: graph = await build_graph(backend, **graph_kwargs) rows = _BUILDERS[which](graph) written = write_csv(rows, csv_path) if csv_path else None return {"rows": rows, "count": len(rows), "csv_path": written, "graph_stats": graph["stats"]} ``` - [ ] **Step 4: Run the tests** — `uv run --frozen pytest tests/test_pid_deliverables.py tests/test_pid_graph.py -v` → PASS. - [ ] **Step 5: Register the three tools** (SECTION 17 → `(7 tools)`) ```python @cad_tool( summary="Instrument index derived from the P&ID graph (tag, function, loop, connections); optional CSV.", cost="read", ) @mcp.tool( annotations={"title": "P&ID: Instrument Index", "readOnlyHint": True}, tags={"pid", "query"} ) async def pid_instrument_index( csv_path: Annotated[str | None, "Write the rows as CSV here (inside ALLOWED_PATHS)"] = None, tolerance: Annotated[float, Field(default=0.5, gt=0)] = 0.5, scope: Annotated[str, "current_space | all"] = "current_space", ctx: Context = None, ) -> dict: """Every instrument bubble with its ISA-5.1 reading and what it connects to. Rows come from `pid_graph`, never from typed-in lists; the drawing is not modified. `csv_path` is the only file this tool writes. """ from engineering.pid.deliverables import deliverable return await deliverable( _backend(ctx), "instrument_index", csv_path, tolerance=tolerance, scope=scope ) @cad_tool( summary="Line list derived from the P&ID graph (number, class, size, spec, from/to); optional CSV.", cost="read", ) @mcp.tool(annotations={"title": "P&ID: Line List", "readOnlyHint": True}, tags={"pid", "query"}) async def pid_line_list( csv_path: Annotated[str | None, "Write the rows as CSV here (inside ALLOWED_PATHS)"] = None, tolerance: Annotated[float, Field(default=0.5, gt=0)] = 0.5, scope: Annotated[str, "current_space | all"] = "current_space", ctx: Context = None, ) -> dict: """Every P&ID line with its number, class, size/service/spec/insulation and endpoints.""" from engineering.pid.deliverables import deliverable return await deliverable(_backend(ctx), "line_list", csv_path, tolerance=tolerance, scope=scope) @cad_tool( summary="Equipment list derived from the P&ID graph (tag, kind, ports, connections); optional CSV.", cost="read", ) @mcp.tool( annotations={"title": "P&ID: Equipment List", "readOnlyHint": True}, tags={"pid", "query"} ) async def pid_equipment_list( csv_path: Annotated[str | None, "Write the rows as CSV here (inside ALLOWED_PATHS)"] = None, tolerance: Annotated[float, Field(default=0.5, gt=0)] = 0.5, scope: Annotated[str, "current_space | all"] = "current_space", ctx: Context = None, ) -> dict: """Every equipment, valve and connector node with its ports and how many lines reach them.""" from engineering.pid.deliverables import deliverable return await deliverable( _backend(ctx), "equipment_list", csv_path, tolerance=tolerance, scope=scope ) ``` Aliases: ```python "pid_instrument_index": ToolAliases(acad=(), synonyms=("instrument index", "instrument list", "loop list", "tag list")), "pid_line_list": ToolAliases(acad=(), synonyms=("line list", "pipe list", "line schedule", "line numbers")), "pid_equipment_list": ToolAliases(acad=(), synonyms=("equipment list", "equipment schedule", "valve list")), ``` Inventory; README `164 tools`. - [ ] **Step 6: Gates and commit** ```bash git add engineering/pid/deliverables.py server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_pid_deliverables.py git commit -m "feat(pid): instrument index, line list and equipment list from the graph" ``` --- ### Task 16: Critique focuses, refine hints, plan/preflight rule **Files:** - Modify: `engineering/plan_spec.py` (`CritiqueFocus`, `ALL_CRITIQUE_FOCUSES`) - Create: `engineering/pid/critique.py` - Modify: `engineering/critique.py` (`_FOCUS_DISPATCH` registration + shared context in `run_critique`) - Modify: `engineering/preflight.py` (`LAYER_SET_INTENT_MISMATCH`), `server.py` (`drawing_plan` warnings; `drawing_critique`/`drawing_finalize` docstrings list the new focuses) - Test: `tests/test_pid_critique.py` **Interfaces:** - Consumes: `build_graph` (Task 14), `LINE_CLASSES` (Task 12), `parse_tag` (Task 6), `Issue`. - Produces: `PID_FOCUSES = ("pid_dangling_line", "pid_duplicate_tag", "pid_incompatible_connection", "pid_untagged_instrument", "pid_illegal_tag", "pid_unconnected_equipment")`; pure checks `issues_for(focus, graph) -> list[Issue]`; async dispatch entries `check(backend, shared)` with `check.needs_shared = True`; `engineering/preflight.py::PID_INTENT_RE`, `intent_reads_as_pid(intent) -> bool`, `pid_plan_warnings(intent, layer_set_id) -> list[str]`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_critique.py """Six P&ID focuses; all silent on a drawing with no P&ID content.""" from __future__ import annotations import pytest from engineering.pid.drawlines import draw_line from engineering.pid.insert import place_symbol from engineering.plan_spec import ALL_CRITIQUE_FOCUSES from engineering.preflight import pid_plan_warnings, preflight_drawing pytestmark = pytest.mark.asyncio PID_FOCUSES = ( "pid_dangling_line", "pid_duplicate_tag", "pid_incompatible_connection", "pid_untagged_instrument", "pid_illegal_tag", "pid_unconnected_equipment", ) def test_focuses_are_in_the_closed_enum(): for focus in PID_FOCUSES: assert focus in ALL_CRITIQUE_FOCUSES async def _clean(backend): pump = await place_symbol(backend, "centrifugal_pump", 100, 100, tag="P-101") cv = await place_symbol(backend, "globe", 160, 106, tag="FCV-101", actuator="diaphragm") fic = await place_symbol( backend, "instrument", 160, 140, tag="FIC-101", type="dcs", location="primary" ) await draw_line( backend, {"handle": pump["handle"], "port": "discharge"}, {"handle": cv["handle"], "port": "in"}, ) await draw_line(backend, {"handle": cv["handle"], "port": "out"}, {"x": 220, "y": 106}) await draw_line( backend, {"handle": fic["handle"]}, {"handle": cv["handle"], "port": "signal"}, line_class="electric", ) return pump, cv, fic async def test_clean_pid_has_no_pid_issues_except_the_free_end(backend): await _clean(backend) issues = await backend.drawing_critique(list(PID_FOCUSES)) focuses = [i.focus for i in issues] assert focuses == ["pid_dangling_line"], "the line to a free point is the only finding" assert issues[0].severity == "error" and "hint" in issues[0].detail async def test_dangling_carries_the_nearest_port_hint(backend): pump, cv, _ = await _clean(backend) await backend.entity_move(cv["handle"], 0.0, 1.0) issues = await backend.drawing_critique(["pid_dangling_line"]) hinted = [i for i in issues if i.detail.get("nearest")] assert hinted and hinted[0].detail["nearest"]["node"] == cv["handle"] assert "pid_line_draw" in hinted[0].detail["hint"] async def test_duplicate_and_illegal_and_untagged(backend): await place_symbol(backend, "instrument", 0, 0, tag="FIC-101") await place_symbol(backend, "instrument", 30, 0, tag="FIC-101") await place_symbol(backend, "instrument", 60, 0, tag="FCI-102") await place_symbol(backend, "instrument", 90, 0) await place_symbol(backend, "gate", 120, 0, tag="FIC-101") issues = await backend.drawing_critique( ["pid_duplicate_tag", "pid_illegal_tag", "pid_untagged_instrument"] ) by_focus = {} for issue in issues: by_focus.setdefault(issue.focus, []).append(issue) assert ( len(by_focus["pid_duplicate_tag"]) == 1 and len(by_focus["pid_duplicate_tag"][0].handles) == 3 ) assert by_focus["pid_duplicate_tag"][0].severity == "error" assert ( len(by_focus["pid_illegal_tag"]) == 1 and by_focus["pid_illegal_tag"][0].severity == "warning" ) assert len(by_focus["pid_untagged_instrument"]) == 1 async def test_incompatible_connection(backend): pump = await place_symbol(backend, "centrifugal_pump", 100, 100, tag="P-101") fic = await place_symbol(backend, "instrument", 100, 160, tag="FIC-1") await draw_line( backend, {"handle": fic["handle"]}, {"handle": pump["handle"], "port": "discharge"}, line_class="electric", ) issues = await backend.drawing_critique(["pid_incompatible_connection"]) assert len(issues) == 1 and issues[0].severity == "error" assert issues[0].detail == { "expected": "signal", "actual": "process", "port": "discharge", "line_class": "electric", "hint": issues[0].detail["hint"], } async def test_unconnected_equipment_is_informational(backend): await place_symbol(backend, "vertical_vessel", 0, 0, tag="V-1") issues = await backend.drawing_critique(["pid_unconnected_equipment"]) assert len(issues) == 1 and issues[0].severity == "info" async def test_mechanical_drawing_is_untouched_by_pid_focuses(backend): from engineering.layers import ensure_engineering_layers await ensure_engineering_layers(backend) await backend.entity_create_circle(0, 0, 20, layer="GEOMETRY") await backend.entity_create_line(-30, 0, 30, 0, layer="CENTER") assert await backend.drawing_critique(list(PID_FOCUSES)) == [] async def test_refine_reports_pid_issues_as_manual_and_moves_nothing(backend): await _clean(backend) before = await backend.entity_count() result = await backend.drawing_refine( max_rounds=1, min_score=100.0, focus=list(PID_FOCUSES), dry_run=True ) assert all(a["status"] == "manual_required" for r in result["rounds"] for a in r["actions"]) assert await backend.entity_count() == before def test_preflight_flags_a_pid_intent_on_the_wrong_layer_set(): result = preflight_drawing( "P&ID for the feed section", {"units": "mm", "part_type": "pid", "dimensions": {}, "tolerance_policy": "none"}, layer_set_id="mech", ) codes = [c.code for c in result.conflicts] assert "LAYER_SET_INTENT_MISMATCH" in codes ok = preflight_drawing( "P&ID for the feed section", {"units": "mm", "part_type": "pid", "dimensions": {}, "tolerance_policy": "none"}, layer_set_id="pid", ) assert "LAYER_SET_INTENT_MISMATCH" not in [c.code for c in ok.conflicts] assert pid_plan_warnings("Bearing housing", "mech") == [] assert pid_plan_warnings("piping and instrumentation diagram", "mech") ``` - [ ] **Step 2: Run to verify they fail** — `uv run --frozen pytest tests/test_pid_critique.py -v` → the enum test fails first. - [ ] **Step 3: Extend the enum** In `engineering/plan_spec.py` add to both `CritiqueFocus` and `ALL_CRITIQUE_FOCUSES`, after `"gdt"`: ```python ("pid_dangling_line",) ("pid_duplicate_tag",) ("pid_incompatible_connection",) ("pid_untagged_instrument",) ("pid_illegal_tag",) ("pid_unconnected_equipment",) ``` - [ ] **Step 4: Write the checks** ```python # engineering/pid/critique.py """Six P&ID focuses over one graph per critique run. Every check is pure over the graph dict; ``run_critique`` builds the graph once into the per-run ``shared`` context so six focuses cost one read. A drawing with no P&ID nodes yields no issues — mechanical sheets are never scored here. """ from __future__ import annotations from typing import TYPE_CHECKING from engineering.plan_spec import Issue from .graph import build_graph from .lines import LINE_CLASSES from .tags import parse_tag if TYPE_CHECKING: from backends.base import AutoCADBackend PID_FOCUSES = ( "pid_dangling_line", "pid_duplicate_tag", "pid_incompatible_connection", "pid_untagged_instrument", "pid_illegal_tag", "pid_unconnected_equipment", ) _KEY = "pid_graph" def _dangling(graph: dict) -> list[Issue]: out = [] for d in graph["dangling"]: nearest = d.get("nearest") if nearest: hint = ( f"pid_line_draw(from_={{'handle': ..., 'port': ...}}, to={{'handle': '{nearest['node']}', " f"'port': '{nearest['port']}'}}) — nearest port {nearest['distance']} mm away" ) else: hint = "no port within reach; connect this end with pid_line_draw or delete the line" out.append( Issue( "error", "pid_dangling_line", f"Line {d['edge']} {d['end']} end at ({d['x']:.2f}, {d['y']:.2f}) connects to nothing.", [d["edge"]], {"end": d["end"], "x": d["x"], "y": d["y"], "nearest": nearest, "hint": hint}, ) ) return out def _duplicate_tags(graph: dict) -> list[Issue]: seen: dict[str, list[str]] = {} for node in graph["nodes"]: if ( node.get("tag") and node["kind"] != "connector" ): # connectors carry free text ("TO P&ID-002") seen.setdefault(node["tag"].upper(), []).append(node["id"]) return [ Issue( "error", "pid_duplicate_tag", f"Tag {tag} is carried by {len(handles)} symbols.", handles, { "tag": tag, "hint": "block_set_attributes(handle, {'TAG'|'FUNC'/'LOOP': ...}) on all but one", }, ) for tag, handles in seen.items() if len(handles) > 1 ] def _incompatible(graph: dict) -> list[Issue]: nodes = {n["id"]: n for n in graph["nodes"]} out = [] for edge in graph["edges"]: cls = LINE_CLASSES.get(edge.get("line_class") or "") if cls is None: continue for ref in (edge["from"], edge["to"]): if not ref or "node" not in ref: continue port = nodes[ref["node"]]["ports"].get(ref["port"], {}) actual = port.get("kind") if actual in ("process", "signal") and actual != cls.kind: out.append( Issue( "error", "pid_incompatible_connection", f"{cls.kind} line {edge['id']} ends on {actual} port {ref['port']} of {ref['node']}.", [edge["id"], ref["node"]], { "expected": cls.kind, "actual": actual, "port": ref["port"], "line_class": edge["line_class"], "hint": f"redraw with a {actual} line class or connect to a {cls.kind} port", }, ) ) return out def _untagged(graph: dict) -> list[Issue]: return [ Issue( "warning", "pid_untagged_instrument", f"Instrument {n['id']} has no tag.", [n["id"]], {"hint": "block_set_attributes(handle, {'FUNC': 'FIC', 'LOOP': '101'})"}, ) for n in graph["nodes"] if n["kind"] == "instrument" and not n.get("tag") ] def _illegal(graph: dict) -> list[Issue]: out = [] for n in graph["nodes"]: if not n.get("tag") or n["kind"] == "connector": continue parsed = parse_tag(n["tag"], kind="instrument" if n["kind"] == "instrument" else "auto") if not parsed["valid"]: out.append( Issue( "warning", "pid_illegal_tag", f"Tag {n['tag']} on {n['id']} is not ISA-5.1: {parsed['errors'][0]}", [n["id"]], {"errors": parsed["errors"], "hint": "block_set_attributes with a valid tag"}, ) ) return out def _unconnected(graph: dict) -> list[Issue]: return [ Issue( "info", "pid_unconnected_equipment", f"{n['kind']} {n.get('tag') or n['id']} has no line on any port.", [n["id"]], {"hint": "informational — connect it with pid_line_draw or leave it if intended"}, ) for n in graph["nodes"] if n["kind"] in ("equipment", "valve") and n["ports"] and not any(p["edges"] for p in n["ports"].values()) ] _CHECKS = { "pid_dangling_line": _dangling, "pid_duplicate_tag": _duplicate_tags, "pid_incompatible_connection": _incompatible, "pid_untagged_instrument": _untagged, "pid_illegal_tag": _illegal, "pid_unconnected_equipment": _unconnected, } def issues_for(focus: str, graph: dict) -> list[Issue]: if not graph["nodes"]: return [] return _CHECKS[focus](graph) def _make(focus: str): async def check(backend: AutoCADBackend, shared: dict) -> list[Issue]: graph = shared.get(_KEY) if graph is None: graph = shared[_KEY] = await build_graph(backend, include_foreign=True) return issues_for(focus, graph) check.needs_shared = True # type: ignore[attr-defined] check.__name__ = f"check_{focus}" return check PID_DISPATCH = {focus: _make(focus) for focus in PID_FOCUSES} ``` In `engineering/critique.py`: after `_FOCUS_DISPATCH = {...}` add ```python from .pid.critique import PID_DISPATCH # noqa: E402 (registered after the mechanical focuses) _FOCUS_DISPATCH.update(PID_DISPATCH) ``` and change the dispatch loop in `run_critique` to ```python shared: dict = {} for f in foci: check = _FOCUS_DISPATCH.get(f) if check is None: ... # unchanged continue if getattr(check, "needs_shared", False): issues.extend(await check(backend, shared)) else: issues.extend(await check(backend)) return issues ``` (`engineering/pid/critique.py` imports `engineering.plan_spec`, not `engineering.critique`, so there is no cycle.) - [ ] **Step 5: Preflight conflict and plan warning** In `engineering/preflight.py` add near the top: ```python PID_INTENT_RE = re.compile( r"\b(p&id|pid|piping and instrumentation|process flow|instrument)\b", re.IGNORECASE ) def intent_reads_as_pid(intent: str) -> bool: return bool(PID_INTENT_RE.search(str(intent or ""))) def pid_plan_warnings(intent: str, layer_set_id: str) -> list[str]: if intent_reads_as_pid(intent) and str(layer_set_id).strip().lower() != "pid": return [ f"intent reads as a P&ID but the layer set is '{layer_set_id}'; pass layer_set_id='pid'" ] return [] ``` (add `import re` to the module imports), and inside `preflight_drawing` after the `parameter_rules` loop: ```python for message in pid_plan_warnings(intent, normalized_layer_set): conflicts.append( PreflightConflict( "LAYER_SET_INTENT_MISMATCH", "layer_set_id", [layer_set_id, "pid"], message ) ) ``` In `server.py::drawing_plan`, replace `return _dc(plan)` with ```python from engineering.preflight import pid_plan_warnings out = _dc(plan) warnings = pid_plan_warnings(intent, layer_set_id) if warnings: out["warnings"] = warnings return out ``` Update the `drawing_critique` `focus` description and the `drawing_finalize` docstring to list the six `pid_*` focuses (both currently enumerate the mechanical ones). - [ ] **Step 6: Run the tests** — `uv run --frozen pytest tests/test_pid_critique.py tests/test_meta_tools.py tests/test_refiner.py tests/test_preflight.py -v` → PASS. - [ ] **Step 7: Commit** ```bash git add engineering/plan_spec.py engineering/pid/critique.py engineering/critique.py engineering/preflight.py server.py tests/test_pid_critique.py git commit -m "feat(pid): six P&ID critique focuses join drawing_finalize; plan/preflight flag a P&ID intent on the wrong layer set" ``` --- ### Task 17: `pid_from_spec` + README showcase render **Files:** - Create: `engineering/pid/spec.py`, `scripts/render_readme_pid.py` - Modify: `server.py` (SECTION 17 → `(8 tools)`), `discovery/aliases.py`, `README.md` (`165 tools`), inventory - Test: `tests/test_pid_from_spec.py` **Interfaces:** - Consumes: `place_symbol`, `draw_line`, `build_graph`, `issues_for`/`PID_FOCUSES`, `resolve`, `transform_port`, `route`, `count_crossings`, backend `transaction_begin/commit/rollback`, `drawing_plan`, `get_plan_spec`. - Produces: `EXAMPLE_SPEC: dict` (the spec §10 example), `validate_spec(spec) -> dict`, `async run_spec(backend, spec, dry_run=False) -> dict`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_from_spec.py """pid_from_spec: one call, one transaction, graph + critique back.""" from __future__ import annotations import copy import pytest import pytest_asyncio from fastmcp import Client import server from engineering.pid.spec import EXAMPLE_SPEC, run_spec, validate_spec pytestmark = pytest.mark.asyncio async def test_example_draws_a_connected_pid(backend): result = await run_spec(backend, EXAMPLE_SPEC) assert set(result["handles"]) == {"P-101", "V-201", "FCV-101", "FIC-101", "OP-1"} graph = result["graph"] assert graph["stats"]["nodes_by_kind"] == { "connector": 1, "equipment": 2, "instrument": 1, "valve": 1, } assert len(graph["edges"]) == 4 and graph["stats"]["dangling"] == 0 assert result["critique"] == [] and result["crossings_total"] == 0 assert backend.get_plan_spec()["layer_set_id"] == "pid" async def test_bad_reference_rolls_back(backend): before = await backend.entity_count() bad = copy.deepcopy(EXAMPLE_SPEC) bad["lines"].append({"from": "P-101.discharge", "to": "V-201.NOPE", "class": "process_major"}) with pytest.raises(ValueError, match=r"lines\[3\]"): await run_spec(backend, bad) assert await backend.entity_count() == before async def test_dry_run_routes_in_memory_and_writes_nothing(backend): before = await backend.entity_count() result = await run_spec(backend, EXAMPLE_SPEC, dry_run=True) assert result["dry_run"] is True and len(result["lines"]) == 4 assert all(len(line["vertices"]) >= 2 for line in result["lines"]) assert await backend.entity_count() == before @pytest.mark.parametrize( "mutate, fragment", [ ( lambda s: s["equipment"].append({"id": "P-101", "symbol": "fan", "x": 0, "y": 0}), "equipment[2].id", ), (lambda s: s["lines"][0].update({"class": "steam"}), "lines[0].class"), (lambda s: s["instruments"][0].pop("x"), "instruments[0].x"), (lambda s: s.update({"bogus": []}), "bogus"), (lambda s: s["connectors"][0].update({"to": "V-201.N1"}), "connectors[0]"), ], ) def test_validation_names_the_path(mutate, fragment): spec = copy.deepcopy(EXAMPLE_SPEC) mutate(spec) with pytest.raises(ValueError) as exc: validate_spec(spec) assert fragment in str(exc.value) @pytest_asyncio.fixture async def client(monkeypatch): monkeypatch.setenv("AUTOCAD_MCP_BACKEND", "ezdxf") async with Client(server.mcp) as connected: await connected.call_tool("drawing_new", {}) yield connected async def test_finalize_scores_the_example_at_ninety_or_better(client): result = (await client.call_tool("pid_from_spec", {"spec": EXAMPLE_SPEC})).structured_content assert result["graph"]["stats"]["dangling"] == 0 final = (await client.call_tool("drawing_finalize", {})).structured_content assert final["score"] >= 90, final assert not [i for i in final.get("critique", []) if str(i.get("focus", "")).startswith("pid_")] ``` - [ ] **Step 2: Run to verify they fail** — ModuleNotFoundError. - [ ] **Step 3: Write the module** ```python # engineering/pid/spec.py """``pid_from_spec``: a whole P&ID from one declarative document, in one transaction.""" from __future__ import annotations import copy from typing import TYPE_CHECKING from .critique import PID_FOCUSES, issues_for from .drawlines import draw_line from .graph import build_graph from .insert import place_symbol from .lines import LINE_CLASSES, count_crossings, route, snap_axis, AXIS from .symbols import resolve, transform_port if TYPE_CHECKING: from backends.base import AutoCADBackend EXAMPLE_SPEC: dict = { "sheet": {"layer_set": "pid", "sheet_size": "A3", "scale": 1.0, "intent": "Feed section P&ID"}, "equipment": [ { "id": "P-101", "symbol": "centrifugal_pump", "x": 100, "y": 100, "rotation": 0, "tag": "P-101", "desc": "Feed pump", }, { "id": "V-201", "symbol": "vertical_vessel", "x": 220, "y": 120, "tag": "V-201", "params": { "width": 20, "height": 40, "nozzles": [ {"name": "N1", "side": "top", "fraction": 0.5}, {"name": "N2", "side": "left", "fraction": 0.3}, ], }, }, ], "valves": [ { "id": "FCV-101", "symbol": "globe", "actuator": "diaphragm", "x": 160, "y": 104, "tag": "FCV-101", "fail": "FC", }, ], "instruments": [ { "id": "FIC-101", "type": "dcs", "location": "primary", "x": 160, "y": 140, "tag": "FIC-101", }, ], "lines": [ { "from": "P-101.discharge", "to": "FCV-101.in", "class": "process_major", "number": "100-P-1001-CS1", }, { "from": "FCV-101.out", "to": "V-201.N2", "class": "process_major", "number": "100-P-1001-CS1", }, {"from": "FIC-101", "to": "FCV-101.signal", "class": "electric"}, ], "connectors": [ { "id": "OP-1", "direction": "out", "x": 300, "y": 120, "tag": "TO P&ID-002", "link": "L-17", "from": "V-201.N1", "class": "process_major", }, ], } _TOP_KEYS = {"sheet", "equipment", "valves", "instruments", "lines", "connectors"} _ITEM_OPTIONS = { "equipment": {"symbol", "x", "y", "rotation", "scale", "tag", "desc", "params", "shape"}, "valves": {"symbol", "x", "y", "rotation", "scale", "tag", "desc", "actuator", "fail"}, "instruments": {"x", "y", "rotation", "scale", "tag", "type", "location"}, "connectors": { "x", "y", "rotation", "scale", "tag", "link", "direction", "from", "to", "class", "number", }, } _LINE_KEYS = { "from", "to", "class", "number", "size", "service", "spec", "insulation", "route", "stub", "label", "arrow", } def _num(value, where): if isinstance(value, bool) or not isinstance(value, (int, float)): raise ValueError(f"{where} must be a number") return float(value) def _ref(value, where, ids: set[str]) -> tuple[str, str | None]: if not isinstance(value, str) or not value: raise ValueError(f"{where} must be 'ID' or 'ID.port'") item_id, _, port = value.partition(".") if item_id not in ids: raise ValueError(f"{where}: unknown id {item_id!r}") return item_id, port or None def validate_spec(spec: dict) -> dict: if not isinstance(spec, dict): raise ValueError("spec must be an object") unknown = set(spec) - _TOP_KEYS if unknown: raise ValueError(f"unknown top-level key(s): {', '.join(sorted(unknown))}") out = {key: copy.deepcopy(spec.get(key) or ([] if key != "sheet" else {})) for key in _TOP_KEYS} if not isinstance(out["sheet"], dict): raise ValueError("sheet must be an object") ids: set[str] = set() for section in ("equipment", "valves", "instruments", "connectors"): items = out[section] if not isinstance(items, list): raise ValueError(f"{section} must be a list") for i, item in enumerate(items): where = f"{section}[{i}]" if not isinstance(item, dict): raise ValueError(f"{where} must be an object") item_id = item.get("id") if not isinstance(item_id, str) or not item_id: raise ValueError(f"{where}.id must be a non-empty string") if item_id in ids: raise ValueError(f"{where}.id {item_id!r} is repeated") ids.add(item_id) bad = set(item) - _ITEM_OPTIONS[section] - {"id"} if bad: raise ValueError(f"{where}: unknown key(s) {', '.join(sorted(bad))}") for key in ("x", "y"): if key not in item: raise ValueError(f"{where}.{key} is required") item[key] = _num(item[key], f"{where}.{key}") if section in ("equipment", "valves") and not isinstance(item.get("symbol"), str): raise ValueError(f"{where}.symbol is required") if section == "connectors": if item.get("direction") not in ("in", "out"): raise ValueError(f"{where}.direction must be 'in' or 'out'") has_from, has_to = "from" in item, "to" in item if item["direction"] == "out" and (not has_from or has_to): raise ValueError(f"{where}: an 'out' connector carries exactly 'from'") if item["direction"] == "in" and (not has_to or has_from): raise ValueError(f"{where}: an 'in' connector carries exactly 'to'") lines = out["lines"] if not isinstance(lines, list): raise ValueError("lines must be a list") for i, line in enumerate(lines): where = f"lines[{i}]" if not isinstance(line, dict): raise ValueError(f"{where} must be an object") bad = set(line) - _LINE_KEYS if bad: raise ValueError(f"{where}: unknown key(s) {', '.join(sorted(bad))}") for key in ("from", "to"): if key not in line: raise ValueError(f"{where}.{key} is required") _ref(line[key], f"{where}.{key}", ids) cls = line.get("class", "process_major") if cls not in LINE_CLASSES: raise ValueError(f"{where}.class must be one of {', '.join(LINE_CLASSES)}") line["class"] = cls for i, conn in enumerate(out["connectors"]): key = "from" if conn["direction"] == "out" else "to" _ref(conn[key], f"connectors[{i}].{key}", ids) cls = conn.get("class", "process_major") if cls not in LINE_CLASSES: raise ValueError(f"connectors[{i}].class must be one of {', '.join(LINE_CLASSES)}") conn["class"] = cls return out def _spec_for(section: str, item: dict): """Resolve the SymbolSpec an item would place (no backend).""" if section == "instruments": return resolve("instrument", type=item.get("type"), location=item.get("location")) if section == "connectors": return resolve("offpage", direction=item["direction"]) return resolve( item["symbol"], actuator=item.get("actuator"), params=item.get("params"), shape=item.get("shape"), ) def _plan_lines(norm: dict) -> list[dict]: """Route every line in memory from catalogue ports — the dry run.""" placed: dict[str, dict] = {} for section in ("equipment", "valves", "instruments", "connectors"): for item in norm[section]: spec = _spec_for(section, item) ports = { p.name: transform_port( p, item["x"], item["y"], item.get("rotation", 0.0), item.get("scale", 1.0) ) for p in spec.ports } placed[item["id"]] = {"family": spec.family, "ports": ports} def endpoint(ref: str) -> dict: item_id, port = ref.partition(".")[::2] entry = placed[item_id] if not port: if len(entry["ports"]) == 1: port = next(iter(entry["ports"])) elif entry["family"] == "instrument": port = "signal" else: raise ValueError(f"{ref}: choose a port: {', '.join(entry['ports'])}") if port not in entry["ports"]: raise ValueError(f"{ref}: no port {port!r}; ports: {', '.join(entry['ports'])}") return entry["ports"][port] def anchor(ep: dict, other: dict): if ep["radius"] > 0: axis = snap_axis(other["x"] - ep["x"], other["y"] - ep["y"]) ux, uy = AXIS[axis] return (ep["x"] + ux * ep["radius"], ep["y"] + uy * ep["radius"]), axis return (ep["x"], ep["y"]), ep["direction_deg"] all_lines = list(norm["lines"]) + [ {"from": c["from"], "to": f"{c['id']}", "class": c["class"]} if c["direction"] == "out" else {"from": f"{c['id']}", "to": c["to"], "class": c["class"]} for c in norm["connectors"] ] planned: list[dict] = [] for i, line in enumerate(all_lines): start, end = endpoint(line["from"]), endpoint(line["to"]) s, sd = anchor(start, end) e, ed = anchor(end, start) vertices = route( s, sd, e, ed, stub=float(line.get("stub", 5.0)), mode=line.get("route", "auto") ) crossings = count_crossings(vertices, [p["vertices"] for p in planned]) planned.append( { "index": i, "from": line["from"], "to": line["to"], "class": line["class"], "vertices": [[x, y] for x, y in vertices], "crossings": crossings, } ) return planned async def run_spec(backend: AutoCADBackend, spec: dict, dry_run: bool = False) -> dict: norm = validate_spec(spec) if dry_run: planned = _plan_lines(norm) return { "dry_run": True, "items": sum( len(norm[s]) for s in ("equipment", "valves", "instruments", "connectors") ), "lines": planned, "crossings_total": sum(p["crossings"] for p in planned), } sheet = norm["sheet"] if backend.get_plan_spec() is None: await backend.drawing_plan( sheet.get("intent", "P&ID"), sheet.get("sheet_size", "A3"), float(sheet.get("scale", 1.0)), sheet.get("layer_set", "pid"), ) begun = await backend.transaction_begin() if not isinstance(begun, dict) or not begun.get("ok"): raise RuntimeError(f"pid_from_spec: could not open a transaction: {begun}") handles: dict[str, str] = {} crossings_total = 0 try: for section in ("equipment", "valves", "instruments", "connectors"): for i, item in enumerate(norm[section]): where = f"{section}[{i}]" try: if section == "instruments": placed = await place_symbol( backend, "instrument", item["x"], item["y"], item.get("rotation", 0.0), item.get("scale", 1.0), item.get("tag"), None, None, None, None, type=item.get("type"), location=item.get("location"), ) elif section == "connectors": placed = await place_symbol( backend, "offpage", item["x"], item["y"], item.get("rotation", 0.0), item.get("scale", 1.0), item.get("tag"), None, None, item.get("link"), None, direction=item["direction"], ) else: placed = await place_symbol( backend, item["symbol"], item["x"], item["y"], item.get("rotation", 0.0), item.get("scale", 1.0), item.get("tag"), item.get("desc"), item.get("fail"), None, None, actuator=item.get("actuator"), params=item.get("params"), shape=item.get("shape"), ) except ValueError as exc: raise ValueError(f"{where}: {exc}") from exc handles[item["id"]] = placed["handle"] def ref(value: str) -> dict: item_id, _, port = value.partition(".") return ( {"handle": handles[item_id], "port": port} if port else {"handle": handles[item_id]} ) all_lines = [(f"lines[{i}]", line) for i, line in enumerate(norm["lines"])] for i, conn in enumerate(norm["connectors"]): if conn["direction"] == "out": all_lines.append( ( f"connectors[{i}]", { "from": conn["from"], "to": conn["id"], "class": conn["class"], "number": conn.get("number"), }, ) ) else: all_lines.append( ( f"connectors[{i}]", { "from": conn["id"], "to": conn["to"], "class": conn["class"], "number": conn.get("number"), }, ) ) for where, line in all_lines: try: drawn = await draw_line( backend, ref(line["from"]), ref(line["to"]), line["class"], line.get("route", "auto"), float(line.get("stub", 5.0)), line.get("number"), line.get("size"), line.get("service"), line.get("spec"), line.get("insulation"), None, bool(line.get("label", True)), line.get("arrow"), ) except ValueError as exc: raise ValueError(f"{where}: {exc}") from exc crossings_total += drawn["crossings"] except Exception: await backend.transaction_rollback() raise await backend.transaction_commit() graph = await build_graph(backend) critique = [issue.to_dict() for focus in PID_FOCUSES for issue in issues_for(focus, graph)] return { "handles": handles, "graph": graph, "critique": critique, "crossings_total": crossings_total, } ``` - [ ] **Step 4: Register the tool** (SECTION 17 → `(8 tools)`) ```python @cad_tool( summary="Draw a whole P&ID from one declarative spec in one transaction; returns graph + critique.", cost="mutate", ) @mcp.tool( annotations={"title": "P&ID: From Spec", "readOnlyHint": False}, tags={"pid", "create", "batch"}, ) async def pid_from_spec( spec: Annotated[ dict, "{sheet, equipment[], valves[], instruments[], lines[], connectors[]} — see the P&ID prompt", ], dry_run: Annotated[bool, "Validate and route in memory; draw nothing"] = False, ctx: Context = None, ) -> dict: """Equipment, valves, instruments and connectors placed, then every line drawn port-to-port, inside one transaction: any bad item rolls the whole sheet back and the error names it (`lines[2].to`). The response carries the graph read back from the drawing and the P&ID critique issues, so the caller sees dangling ends or duplicate tags in the same round trip. `dry_run` returns the planned vertices and crossings without touching the drawing. """ from engineering.pid.spec import run_spec await ctx.info("P&ID from spec" + (" (dry run)" if dry_run else "")) return await run_spec(_backend(ctx), spec, dry_run) ``` Alias: ```python "pid_from_spec": ToolAliases( acad=(), synonyms=("whole p&id in one call", "p&id from json", "draw the process from a spec", "batch p&id"), ), ``` Inventory; README `165 tools`. - [ ] **Step 5: README showcase render script** ```python # scripts/render_readme_pid.py """Render the P&ID the README shows, using only the server's own tools. AUTOCAD_MCP_BACKEND=ezdxf uv run --frozen python scripts/render_readme_pid.py """ from __future__ import annotations import argparse import asyncio import sys from pathlib import Path ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(ROOT)) DEFAULT_OUTPUT = ROOT / "docs" / "assets" / "pid-showcase.png" async def render(output: Path) -> dict: from backends.ezdxf_backend import EzdxfBackend from engineering.layers import apply_layer_set from engineering.pid.spec import EXAMPLE_SPEC, run_spec backend = EzdxfBackend() await backend.connect() try: await backend.drawing_new() await apply_layer_set(backend, "pid") result = await run_spec(backend, EXAMPLE_SPEC) png = await backend.view_screenshot() output.parent.mkdir(parents=True, exist_ok=True) output.write_bytes(png) return { "output": str(output), "bytes": len(png), "dangling": result["graph"]["stats"]["dangling"], "critique": len(result["critique"]), } finally: await backend.disconnect() def main() -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--output", type=Path, default=DEFAULT_OUTPUT) args = parser.parse_args() print(asyncio.run(render(args.output))) return 0 if __name__ == "__main__": raise SystemExit(main()) ``` Run it once (`uv run --frozen python scripts/render_readme_pid.py`) and commit the PNG. - [ ] **Step 6: Gates and commit** Run: `uv run --frozen pytest tests/test_pid_from_spec.py tests/test_discovery_aliases.py tests/test_tool_inventory.py tests/test_release_consistency.py -v` ```bash git add engineering/pid/spec.py scripts/render_readme_pid.py docs/assets/pid-showcase.png server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_pid_from_spec.py git commit -m "feat(pid): pid_from_spec — a whole P&ID in one transaction, graph and critique returned" ``` --- ### Task 18: Tool packs (`TOOL_PACKS`) **Files:** - Modify: `config.py` (`tool_packs`), `server.py` (`TOOL_PACK_NAMES`, `PACK_TOOL_NAMES`, `_enabled_packs`, `_profile_enabled_names`, `_apply_tool_profile`, `system_about`, the `unknown_tool` rejection text, `LEAN_TOOL_NAMES` + 3), `.env.example`, `README.md` (Configuration table row) - Test: `tests/test_tool_packs.py` **Interfaces:** - Produces: `config.settings.tool_packs: str` (default `"all"`); `server.TOOL_PACK_NAMES = ("core", "pid")`; `server.PACK_TOOL_NAMES = {"pid": frozenset({...9 tools...})}`; `server._enabled_packs() -> tuple[set[str], list[str]]` (enabled, ignored); `_active_tool_profile["tool_packs"] = {"enabled": [...], "available": [...], "ignored": [...]}`; `system_about()["tool_packs"]`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_tool_packs.py """TOOL_PACKS: advertise only the vertical packs a client uses; core is always on.""" from __future__ import annotations import logging import pytest import pytest_asyncio import config import server pytestmark = pytest.mark.asyncio PID_TOOLS = { "pid_symbol_list", "pid_symbol_insert", "pid_line_draw", "pid_tag_parse", "pid_graph", "pid_instrument_index", "pid_line_list", "pid_equipment_list", "pid_from_spec", } @pytest_asyncio.fixture(autouse=True) async def _restore(monkeypatch): yield monkeypatch.setattr(config.settings, "tool_packs", "all") await server._apply_tool_profile("full") def test_pack_registry_names_real_tools_and_only_pid_ones(): assert server.TOOL_PACK_NAMES == ("core", "pid") assert server.PACK_TOOL_NAMES["pid"] == frozenset(PID_TOOLS) async def test_core_only_hides_every_pid_tool_but_keeps_block_define(monkeypatch): monkeypatch.setattr(config.settings, "tool_packs", "core") info = await server._apply_tool_profile("full") disabled = set(info["disabled_tools"]) assert PID_TOOLS <= disabled assert "block_define" not in disabled and "entity_set_xdata" not in disabled assert info["tool_packs"] == {"enabled": ["core"], "available": ["core", "pid"], "ignored": []} async def test_all_is_the_default_and_enables_pid(monkeypatch): monkeypatch.setattr(config.settings, "tool_packs", "all") info = await server._apply_tool_profile("full") assert not (PID_TOOLS & set(info["disabled_tools"])) assert info["tool_packs"]["enabled"] == ["core", "pid"] async def test_unknown_pack_is_ignored_with_a_warning_and_core_cannot_be_dropped( monkeypatch, caplog ): monkeypatch.setattr(config.settings, "tool_packs", "pid,mech3000") with caplog.at_level(logging.WARNING, logger="autocad_mcp"): info = await server._apply_tool_profile("full") assert info["tool_packs"] == { "enabled": ["core", "pid"], "available": ["core", "pid"], "ignored": ["mech3000"], } assert "mech3000" in caplog.text async def test_lean_intersects_with_packs(monkeypatch): assert {"pid_symbol_insert", "pid_line_draw", "pid_graph"} <= server.LEAN_TOOL_NAMES monkeypatch.setattr(config.settings, "tool_packs", "core") info = await server._apply_tool_profile("lean") assert ( "pid_graph" in info["disabled_tools"] and "entity_create_line" not in info["disabled_tools"] ) async def test_system_about_reports_packs(): from fastmcp import Client async with Client(server.mcp) as client: about = (await client.call_tool("system_about", {})).structured_content assert about["tool_packs"]["available"] == ["core", "pid"] ``` - [ ] **Step 2: Run to verify they fail** — `AttributeError: module 'server' has no attribute 'TOOL_PACK_NAMES'`. - [ ] **Step 3: Implement** `config.py`, next to `tool_profile`: ```python # Tool packs: "all" (default) advertises every vertical pack; a comma # list ("core,pid") advertises only those. core cannot be disabled. self.tool_packs: str = os.environ.get("TOOL_PACKS", "all").lower().strip() ``` `server.py`, after `SOLID_TOOL_NAMES`: ```python # Tool packs: vertical domains a client can opt out of advertising. `core` is # everything not claimed by another pack and is always on. ~300 tools would # push the full catalog to ~80k idle tokens; a client that only drafts # mechanically should not pay for the P&ID surface. TOOL_PACK_NAMES = ("core", "pid") PACK_TOOL_NAMES: dict[str, frozenset[str]] = { "pid": frozenset( { "pid_symbol_list", "pid_symbol_insert", "pid_line_draw", "pid_tag_parse", "pid_graph", "pid_instrument_index", "pid_line_list", "pid_equipment_list", "pid_from_spec", } ), } def _enabled_packs() -> tuple[set[str], list[str]]: raw = (config.settings.tool_packs or "all").lower() requested = [p.strip() for p in raw.split(",") if p.strip()] if not requested or "all" in requested: return set(TOOL_PACK_NAMES), [] enabled = {"core"} ignored: list[str] = [] for pack in requested: if pack in TOOL_PACK_NAMES: enabled.add(pack) else: ignored.append(pack) log.warning( "Unknown TOOL_PACKS entry %r ignored. Valid packs: %s.", pack, ", ".join(TOOL_PACK_NAMES), ) return enabled, ignored ``` Change `_profile_enabled_names`: ```python def _profile_enabled_names(profile: str, registered: set[str]) -> set[str]: packs, _ignored = _enabled_packs() hidden_by_pack = set().union( *(names for pack, names in PACK_TOOL_NAMES.items() if pack not in packs) ) enabled = set(registered) - hidden_by_pack if profile == "lean": enabled &= LEAN_TOOL_NAMES if not config.settings.enable_3d: enabled -= SOLID_TOOL_NAMES return enabled ``` In `_apply_tool_profile`, after building `_active_tool_profile`, add: ```python packs, ignored = _enabled_packs() _active_tool_profile["tool_packs"] = { "enabled": sorted(packs), "available": list(TOOL_PACK_NAMES), "ignored": ignored, } ``` In `system_about`, add `"tool_packs": (_active_tool_profile or {}).get("tool_packs") or {"enabled": sorted(_enabled_packs()[0]), "available": list(TOOL_PACK_NAMES), "ignored": _enabled_packs()[1]}`. Add the three pid tools to `LEAN_TOOL_NAMES`. Find the `unknown_tool` rejection text (`server.py` ~line 4059: "the active TOOL_PROFILE / ENABLE_3D gate") and make it "the active TOOL_PROFILE / TOOL_PACKS / ENABLE_3D gate". `.env.example`, after `TOOL_PROFILE=full`: ``` # Tool packs: all (default) | comma list of core,pid — advertise only the # vertical packs this client uses. core is always on. TOOL_PACKS=all ``` README Configuration table: a row `| TOOL_PACKS | all | Vertical packs to advertise: core,pid (core always on) |` matching the table's existing column layout. - [ ] **Step 4: Run** — `uv run --frozen pytest tests/test_tool_packs.py tests/test_tool_profiles.py tests/test_tool_registry.py -v` → PASS. - [ ] **Step 5: Commit** ```bash git add config.py server.py .env.example README.md tests/test_tool_packs.py git commit -m "feat(server): TOOL_PACKS — advertise only the vertical packs a client uses" ``` --- ### Task 19: `pid_tag_parse` tool, prompt rewrite, two resources, catalogue contact sheet **Files:** - Modify: `server.py` (SECTION 17 → `(9 tools)`; `prompt_pid_diagram` rewritten; two `@mcp.resource`), `discovery/aliases.py`, `README.md` (`166 tools`, `8 resources`), inventory - Create: `scripts/render_pid_catalog.py`, `docs/assets/pid-catalog.png` - Test: `tests/test_pid_resources_prompt.py` **Interfaces:** - Produces: MCP tool `pid_tag_parse(tag, kind="auto", equipment_prefixes=None)`; resources `autocad://pid/symbols`, `autocad://standards/isa51`; `scripts/render_pid_catalog.py::render(output) -> dict`. - [ ] **Step 1: Write the failing tests** ```python # tests/test_pid_resources_prompt.py from __future__ import annotations import json import pytest import pytest_asyncio from fastmcp import Client import server pytestmark = pytest.mark.asyncio @pytest_asyncio.fixture async def client(monkeypatch): monkeypatch.setenv("AUTOCAD_MCP_BACKEND", "ezdxf") async with Client(server.mcp) as connected: yield connected async def test_tag_parse_tool(client): out = (await client.call_tool("pid_tag_parse", {"tag": "10-FIC-101A"})).structured_content assert out["valid"] is True and out["description"] == "Flow Indicating Controller" out = ( await client.call_tool( "pid_tag_parse", {"tag": "Q-1", "kind": "equipment", "equipment_prefixes": {"Q": "quench"}}, ) ).structured_content assert out["equipment_kind"] == "quench" async def test_resources_serve_the_catalogue_and_the_letter_tables(client): uris = {str(r.uri) for r in await client.list_resources()} assert {"autocad://pid/symbols", "autocad://standards/isa51"} <= uris symbols = json.loads((await client.read_resource("autocad://pid/symbols"))[0].text) assert symbols["catalog_version"] == "1" and any( s["symbol"] == "gate" for s in symbols["symbols"] ) isa = json.loads((await client.read_resource("autocad://standards/isa51"))[0].text) assert isa["first_letters"]["F"] == "Flow" and "rules" in isa async def test_prompt_walks_the_tool_workflow(client): prompt = await client.get_prompt( "prompt_pid_diagram", {"project_name": "Unit 7", "revision": "B"} ) text = "\n".join(m.content.text for m in prompt.messages) for needle in ( "drawing_plan", 'layer_set_id="pid"', "pid_symbol_insert", "pid_line_draw", "pid_graph", "drawing_critique", "drawing_finalize", "pid_from_spec", "Unit 7", "Ø10", ): assert needle in text, needle assert "circle with triangle" not in text, "no hand-drawn symbols" def test_catalog_contact_sheet_renders_every_symbol(tmp_path): pytest.importorskip("matplotlib") from scripts.render_pid_catalog import render result = render(tmp_path / "sheet.png") assert result["symbols"] >= 100 and result["bytes"] > 10_000 ``` - [ ] **Step 2: Run to verify they fail**. - [ ] **Step 3: Tool, resources, prompt** SECTION 17 → `(9 tools)`; add: ```python @cad_tool( summary="Parse an ISA-5.1 instrument tag or an equipment tag; explains every letter.", cost="read", ) @mcp.tool(annotations={"title": "P&ID: Parse Tag", "readOnlyHint": True}, tags={"pid", "query"}) async def pid_tag_parse( tag: Annotated[str, "e.g. FIC-101, 10-PDT-203A, P-101A"], kind: Annotated[str, "auto | instrument | equipment"] = "auto", equipment_prefixes: Annotated[ dict | None, "Replace the default equipment prefix map, e.g. {'Q': 'quench tower'}" ] = None, ctx: Context = None, ) -> dict: """ISA-5.1-2009 Table 4.1 as a grammar: first letter, modifier, readout and output functions, trailing high/low, loop number and suffix — with the standard's description ("Flow Indicating Controller") or the first offending letter. Equipment prefixes are not standardised, so the default map (P pump, V vessel, E exchanger, ...) can be replaced per call. """ from engineering.pid.tags import parse_tag return parse_tag(tag, kind, equipment_prefixes) ``` Alias: `"pid_tag_parse": ToolAliases(acad=(), synonyms=("what does FIC mean", "isa 5.1 tag", "instrument tag letters", "decode the tag", "loop number"))`. Resources (in the RESOURCES block): ```python @mcp.resource( "autocad://pid/symbols", name="P&ID Symbol Catalogue", description="Every P&ID symbol pid_symbol_insert can place: families, variants, ports, parameters", mime_type="application/json", annotations={"readOnlyHint": True}, tags={"pid"}, ) async def resource_pid_symbols() -> str: from engineering.pid.symbols import CATALOG_VERSION, list_symbols return json.dumps({"catalog_version": CATALOG_VERSION, "symbols": list_symbols()}, indent=2) @mcp.resource( "autocad://standards/isa51", name="ISA-5.1 Identification Letters", description="ISA-5.1-2009 Table 4.1 letter tables, the tag grammar and its disambiguation rules", mime_type="application/json", annotations={"readOnlyHint": True}, tags={"pid", "standards"}, ) async def resource_isa51() -> str: from engineering.pid.tags import describe_tables return json.dumps(describe_tables(), indent=2) ``` Replace the body of `prompt_pid_diagram` with: ```python return f"""You are creating a P&ID for '{project_name}' ({revision}) with the server's P&ID tools. Never hand-draw a symbol from lines and circles: every valve, pump, vessel and instrument is a catalogue block with ports, and every line is drawn port-to-port. REFERENCE SIZES (symbol-local mm at A3/A1 paper scale): instrument bubble Ø10, valve body 8×4, pump Ø8, grid 5. Keep equipment ≥ 40 mm apart so line numbers and tags have room. WORKFLOW 1. drawing_plan(intent="{project_name} P&ID", sheet_size="A3", layer_set_id="pid") 2. drawing_apply_iso_layers("pid") 3. pid_symbol_list() — read the catalogue once; pid_tag_parse(tag) when unsure about ISA-5.1 letters 4. pid_symbol_insert(symbol, x, y, tag=...) for each item: equipment tag P-101 / V-201 / E-301 (desc="Feed pump") valves symbol=gate|globe|ball|... with actuator=diaphragm|piston|motor|solenoid|hand and fail=FO|FC|FL instruments symbol="instrument", type=discrete|dcs|computer|plc, location=field|primary|auxiliary|..., tag="FIC-101" vessels params={{"width", "height", "nozzles": [{{"name": "N1", "side": "top", "fraction": 0.5}}]}} Each call returns the block's ports in WCS — use them, never guess coordinates. 5. pid_line_draw(from_={{"handle", "port"}}, to={{"handle", "port"}}, line_class=process_major|process_minor| utility|pneumatic|electric|hydraulic|capillary|data, size="100", service="P", spec="CS1") A bubble's radial port needs no name. Read `crossings` and `port_reuse` in the response. 6. pid_graph() — `dangling` must be empty and `stats.confidence_min` 1.0 for a drawing you made. 7. drawing_critique(focus=None) — must return [] (pid_dangling_line, pid_duplicate_tag, pid_incompatible_connection, pid_untagged_instrument, pid_illegal_tag, pid_unconnected_equipment are part of it). 8. pid_instrument_index() / pid_line_list() / pid_equipment_list() for the deliverables. 9. drawing_finalize(save_path=...) — the score is the objective quality metric. ONE-CALL ALTERNATIVE: pid_from_spec(spec) draws the whole sheet in one transaction from {{sheet, equipment[], valves[], instruments[], lines[], connectors[]}} and returns the graph and the critique; use dry_run=true first to see the planned routes and crossings. OFF-PAGE: pid_symbol_insert("offpage", direction="out"|"in", tag="TO P&ID-002", link="L-17"); two connectors sharing a link are paired by pid_graph. """ ``` Regenerate the inventory (now 8 resources); README snapshot `166 tools · 8 resources`. - [ ] **Step 4: Contact-sheet script** ```python # scripts/render_pid_catalog.py """Render every catalogue symbol on one labelled sheet (docs/assets/pid-catalog.png). uv run --frozen python scripts/render_pid_catalog.py """ from __future__ import annotations import argparse import asyncio import sys from pathlib import Path ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(ROOT)) DEFAULT_OUTPUT = ROOT / "docs" / "assets" / "pid-catalog.png" CELL = 80.0 # the tallest vessel spans 66 mm with its nozzle stubs COLUMNS = 10 async def _render(output: Path) -> dict: from backends.ezdxf_backend import EzdxfBackend from engineering.layers import apply_layer_set from engineering.pid.insert import insert_symbol from engineering.pid.symbols import all_specs backend = EzdxfBackend() await backend.connect() try: await backend.drawing_new() await apply_layer_set(backend, "pid") specs = all_specs() for index, spec in enumerate(specs): col, row = index % COLUMNS, index // COLUMNS x, y = col * CELL, -row * CELL layer = "PROCESS-PIPING-MAIN" if spec.family == "marker" else None await insert_symbol(backend, spec, x, y, 0.0, 1.0, {}, layer) await backend.entity_create_text( spec.name.replace("PID_", ""), x - CELL / 2 + 2, y - CELL / 2 + 2, 1.6, layer="TEXT-NOTES", ) png = await backend.view_screenshot() output.parent.mkdir(parents=True, exist_ok=True) output.write_bytes(png) return {"output": str(output), "symbols": len(specs), "bytes": len(png)} finally: await backend.disconnect() def render(output: Path) -> dict: return asyncio.run(_render(output)) def main() -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--output", type=Path, default=DEFAULT_OUTPUT) print(render(parser.parse_args().output)) return 0 if __name__ == "__main__": raise SystemExit(main()) ``` Run it and commit the PNG. (`scripts/` has no `__init__.py`; the test imports `scripts.render_pid_catalog` — add an empty `scripts/__init__.py` if the import fails, and keep it out of the wheel: check `pyproject.toml`'s hatch `include` list does not pick it up.) - [ ] **Step 5: Run** — `uv run --frozen pytest tests/test_pid_resources_prompt.py tests/test_tool_inventory.py tests/test_release_consistency.py tests/test_discovery_aliases.py -v`. - [ ] **Step 6: Commit** ```bash git add server.py discovery/aliases.py docs/tool-inventory.json README.md scripts/render_pid_catalog.py docs/assets/pid-catalog.png tests/test_pid_resources_prompt.py git commit -m "feat(pid): pid_tag_parse, catalogue and ISA-5.1 resources, prompt rewritten onto the tools" ``` --- ### Task 20: Benchmark matrix v4 (`pid_roundtrip`) + three correctness checks **Files:** - Create: `benchmarks/tasks_v4.py`, `tests/test_benchmark_v4.py` - Modify: `benchmarks/adapters/autocad_mcp_pro.py` (`_task_pid_roundtrip`), `benchmarks/run_competitors.py` and `benchmarks/render_matrix_chart.py` (import from `tasks_v4`), `benchmarks/correctness_suite.py` (+3 checks), `scripts/check_doc_numbers.py` (task-count pattern → v4), `CLAUDE.md`/`README.md`/`benchmarks/README.md` numbers (26 → 29 checks; 15 → 16 tasks) **Interfaces:** - Produces: `benchmarks.tasks_v4.NEW_TASKS_V4`, `TASKS_V4 = TASKS_V3 + NEW_TASKS_V4`, `MATRICES = {"v2", "v3", "v4"}`, `DEFAULT_MATRIX = "v4"`, `task_by_id`; correctness checks `pid_block_define_attdef_roundtrip`, `pid_tag_parse_fic`, `pid_graph_edge_count` under group `"P&ID"`. - [ ] **Step 1: Write the failing test** ```python # tests/test_benchmark_v4.py from __future__ import annotations from benchmarks.tasks_v3 import TASKS_V3 from benchmarks.tasks_v4 import DEFAULT_MATRIX, MATRICES, NEW_TASKS_V4, TASKS_V4, task_by_id def test_v4_extends_v3_without_touching_it(): assert TASKS_V4[: len(TASKS_V3)] == TASKS_V3 assert [t.task_id for t in NEW_TASKS_V4] == ["pid_roundtrip"] assert MATRICES["v3"] == TASKS_V3 and MATRICES["v4"] == TASKS_V4 and DEFAULT_MATRIX == "v4" assert task_by_id("pid_roundtrip").category == "pid" def test_the_adapter_implements_every_v4_task(): from benchmarks.adapters.autocad_mcp_pro import AutoCADMCPProAdapter for task in TASKS_V4: assert hasattr(AutoCADMCPProAdapter, f"_task_{task.task_id}"), task.task_id def test_correctness_suite_has_the_pid_checks(): from benchmarks.correctness_suite import CHECKS for name in ("pid_block_define_attdef_roundtrip", "pid_tag_parse_fic", "pid_graph_edge_count"): assert CHECKS[name][1] == "P&ID" assert len(CHECKS) == 29 ``` - [ ] **Step 2: Run to verify it fails** — ModuleNotFoundError. - [ ] **Step 3: Write the matrix, the adapter task, the checks** ```python # benchmarks/tasks_v4.py """The v4 task matrix: v3 plus the P&ID round trip (v1.6, track A). ``pid_roundtrip`` draws a five-item P&ID through the same code the tools call and reads it back: the edge count, the absence of dangling ends and the instrument index are each derived independently of the drawing code. The published competitor reports were never asked this question; the chart shows ``not_run`` for them, not zero. """ from __future__ import annotations from benchmarks.tasks_v2 import TASKS_V2, TaskSpec from benchmarks.tasks_v3 import TASKS_V3 NEW_TASKS_V4: tuple[TaskSpec, ...] = ( TaskSpec( "pid_roundtrip", "pid", "Draw a P&ID from a spec and read back a matching graph, index and line list", 1.0, ), ) TASKS_V4: tuple[TaskSpec, ...] = TASKS_V3 + NEW_TASKS_V4 MATRICES: dict[str, tuple[TaskSpec, ...]] = {"v2": TASKS_V2, "v3": TASKS_V3, "v4": TASKS_V4} DEFAULT_MATRIX = "v4" def task_by_id(task_id: str, matrix: str = DEFAULT_MATRIX) -> TaskSpec: for task in MATRICES[matrix]: if task.task_id == task_id: return task raise KeyError(f"{task_id} is not in matrix {matrix}") ``` Adapter method (in `benchmarks/adapters/autocad_mcp_pro.py`, after `_task_measure_from_handle`): ```python async def _task_pid_roundtrip(self): """Five symbols, four lines, read back with nothing dangling.""" from engineering.layers import apply_layer_set from engineering.pid.deliverables import instrument_index, line_list from engineering.pid.spec import EXAMPLE_SPEC, run_spec await apply_layer_set(self.backend, "pid") result = await run_spec(self.backend, EXAMPLE_SPEC) graph = result["graph"] index = instrument_index(graph) lines = line_list(graph) passed = ( len(graph["nodes"]) == 5 and len(graph["edges"]) == 4 and graph["stats"]["dangling"] == 0 and graph["stats"]["confidence_min"] == 1.0 and result["critique"] == [] and [r["tag"] for r in index] == ["FIC-101"] and index[0]["connected_to"] == "FCV-101" and sorted(r["line_number"] for r in lines if r["line_number"]) == ["100-P-1001-CS1", "100-P-1001-CS1"] ) return ( passed, { "nodes": len(graph["nodes"]), "edges": len(graph["edges"]), "dangling": graph["stats"]["dangling"], "confidence_min": graph["stats"]["confidence_min"], "critique_issues": len(result["critique"]), }, [], ) ``` `run_competitors.py` and `render_matrix_chart.py`: change `from benchmarks.tasks_v3 import ...` to `from benchmarks.tasks_v4 import ...` (the chart renders `TASKS_V4` rows; competitor rows for the v3/v4 additions render `not_run` exactly as they do today for v3's five). Correctness checks (append to `benchmarks/correctness_suite.py` before `CHECKS`): ```python async def pid_block_define_attdef_roundtrip(): b = await _b() await b.block_define( "PID_T", [{"type": "line", "x1": 0, "y1": 0, "x2": 8, "y2": 0}], [{"tag": "TAG", "x": 0, "y": 3, "height": 2.5}], ) ref = await b.block_insert("PID_T", 5, 5, attributes={"TAG": "HV-1"}) return await b.block_get_attributes(ref.handle) == {"TAG": "HV-1"} async def pid_tag_parse_fic(): from engineering.pid.tags import parse_tag return parse_tag("FIC-101")["description"] == "Flow Indicating Controller" async def pid_graph_edge_count(): from engineering.pid.spec import EXAMPLE_SPEC, run_spec b = await _b() result = await run_spec(b, EXAMPLE_SPEC) return len(result["graph"]["edges"]) == 4 and result["graph"]["stats"]["dangling"] == 0 ``` and the three `CHECKS` entries with group `"P&ID"`. - [ ] **Step 4: Doc numbers** `uv run --frozen python scripts/check_doc_numbers.py` lists every drift. Update: README "26 deterministic headless checks" → 29 and the A/B table (see Task 21 for the table re-run); `benchmarks/README.md` "26 checks, ezdxf backend" → 29; `CLAUDE.md` "`correctness_suite.py` (26 checks)" → 29 and "`benchmarks/tasks_v3.py` is the current set (15 tasks; `--matrix v2` reproduces the v1.4 ten)" → "`benchmarks/tasks_v4.py` is the current set (16 tasks; `--matrix v3` / `--matrix v2` reproduce the earlier sets)"; in `scripts/check_doc_numbers.py` change the `v3_tasks` source to `from benchmarks.tasks_v4 import TASKS_V4` and the CLAUDE.md pattern to `r"the current set \((\d+) tasks"` (unchanged text shape). - [ ] **Step 5: Run** — `uv run --frozen pytest tests/test_benchmark_v4.py tests/test_benchmark_v3.py tests/test_benchmark_v2.py -v` and `uv run --frozen python benchmarks/run_competitors.py --matrix v4 --server autocad-mcp-pro --task pid_roundtrip` → `pid_roundtrip: pass`. - [ ] **Step 6: Commit** ```bash git add benchmarks/tasks_v4.py benchmarks/adapters/autocad_mcp_pro.py benchmarks/run_competitors.py benchmarks/render_matrix_chart.py benchmarks/correctness_suite.py scripts/check_doc_numbers.py CLAUDE.md README.md benchmarks/README.md tests/test_benchmark_v4.py git commit -m "bench: matrix v4 adds pid_roundtrip; correctness suite grows to 29 with three P&ID checks" ``` --- ### Task 21: Live COM smoke, documentation, A/B re-run, full gate **Files:** - Create: `scripts/smoke_pid_com.py` - Modify: `CLAUDE.md` (§17 P&ID, contracts, `TOOL_PACKS`, reader rules), `README.md` (What-you-get row, catalogue + showcase images, token table per pack, A/B table), `CHANGELOG.md` (`Unreleased` entry), `docs/superpowers/specs/2026-09-15-v1.6-pid-design.md` (status line → implemented) - [ ] **Step 1: Live COM smoke script** ```python # scripts/smoke_pid_com.py """Exercise the P&ID track once against the live AutoCAD on this machine. Creates a NEW document (never touches the operator's open drawing), then runs block_define -> pid_symbol_insert -> xdata -> pid_line_draw -> pid_graph and prints the graph stats. Exit 2 when no AutoCAD is reachable. AUTOCAD_MCP_BACKEND=com uv run --frozen python scripts/smoke_pid_com.py """ from __future__ import annotations import asyncio import json import sys from pathlib import Path ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(ROOT)) async def main() -> int: try: from backends.com_backend import ComBackend except ImportError as exc: print(f"pywin32 not available: {exc}") return 2 backend = ComBackend() try: await backend.connect() except Exception as exc: print(f"no live AutoCAD reachable: {exc}") return 2 try: await backend.drawing_new() status = await backend.system_status() from engineering.layers import apply_layer_set from engineering.pid.spec import EXAMPLE_SPEC, run_spec await apply_layer_set(backend, "pid") result = await run_spec(backend, EXAMPLE_SPEC) got = await backend.entity_get_xdata(result["handles"]["P-101"], "ACADMCP_PID") report = { "autocad": status.get("autocad_version") or status.get("cad_progid"), "graph_stats": result["graph"]["stats"], "critique_issues": len(result["critique"]), "xdata_chunks": len(got["xdata"].get("ACADMCP_PID", [])), } print(json.dumps(report, indent=2)) return 0 if result["graph"]["stats"]["dangling"] == 0 else 1 finally: await backend.disconnect() if __name__ == "__main__": raise SystemExit(asyncio.run(main())) ``` Run it with AutoCAD open: `AUTOCAD_MCP_BACKEND=com uv run --frozen python scripts/smoke_pid_com.py`. Paste the printed report into the CHANGELOG entry. If it exits 2, the CHANGELOG says the COM paths are fake-tested only. If a COM call fails (a wrong ActiveX member name, a variant shape), fix the backend, re-run the fake tests, and re-run the smoke until it passes — that is the point of running it. - [ ] **Step 2: CLAUDE.md** Add to the tool sections list: ``` 17. P&ID (9 tools, pack `pid`): `pid_symbol_list/insert` (ISO 10628-2 + ISA-5.1 catalogue authored in code, real blocks with TAG attributes, named ports, ACADMCP_PID XDATA), `pid_line_draw` (port-to-port orthogonal routing, ISA-5.1 line classes → layer/linetype/marker blocks, line numbers), `pid_tag_parse` (ISA-5.1 grammar), the reader `pid_graph` and its deliverables `pid_instrument_index/line_list/equipment_list`, and `pid_from_spec` (whole sheet, one transaction). Six `pid_*` critique focuses run under `focus=None` and inside `drawing_finalize`; they return nothing on a drawing without P&ID nodes. ``` Add to Key Conventions: ``` - **Generic contracts added in 1.6:** `block_define(name, entities, attdefs)` (typed primitives + ATTDEFs, both engines; `overwrite` replaces contents so INSERTs keep the name) and `entity_get/set_xdata` (typed by group code; 255-char strings, 16 KB per entity, enforced before writing). - **P&ID reader rules:** `pid_graph` never modifies the drawing, reads geometry from the drawing (an INSERT's real rotation/scale) rather than the catalogue, and reports `source`/`confidence` per node (catalog 1.0, xdata 0.95, heuristic 0.6, inferred 0.3) — never raised by agreement between heuristics. Read `stats.confidence_min` before trusting a foreign drawing. - **`TOOL_PACKS=all|core,pid`** advertises only the packs a client uses; `core` is always on; `system_about` reports `tool_packs`. ``` - [ ] **Step 3: README** - "What you get" table: a row `| P&ID | catalogue blocks with ports and tags (ISO 10628-2 / ISA-5.1), port-to-port lines with ISA-5.1 classes and line numbers, `pid_graph` reads any P&ID back with confidence, instrument index / line list / equipment list, `pid_from_spec` one-call sheets |`. - Under the hero image add the catalogue and P&ID showcase images (`docs/assets/pid-catalog.png`, `docs/assets/pid-showcase.png`) with a one-line caption each, the same `` style as the hero. - Token table: measure `uv run --frozen python benchmarks/token_suite.py` once with `TOOL_PACKS=core` and once with the default and add a row `| TOOL_PACKS=core (full profile) | | |`. - A/B table: run `uv run --frozen python benchmarks/compare_versions.py v1.5.1`; rewrite the rows as `v1.5.1 *(baseline)*` and `v1.6.0-dev *(this branch)*` with the printed numbers (expect `26 / 29` → `29 / 29`, three `miss → pass`), and update the two `v1.5.0`/`v1.5.1` regexes in `scripts/check_doc_numbers.py` to the new labels so the gate keeps matching. - Snapshot line: `166 tools · 8 resources · 5 prompt templates · collected tests` — take `` from `uv run --frozen pytest --collect-only -q | tail -1` on this machine, and note in the CHANGELOG if it differs from the number CI reports on Linux (the 1369 vs 1363 drift found on 2026-09-15 predates this track; resolve it now by using the CI number if CI is green, else this machine's). - [ ] **Step 4: CHANGELOG `Unreleased`** Headings: **Added** (the nine tools with the measured refusals from the tests — the 255-char/16 KB XDATA limits, the stub refusal, the crossing count, the confidence ladder, the `*U1` fix), **Fixed** (`block_insert` with attributes lost its ATTRIBs headlessly — Task 1, with the reproduction), **Changed** (`TOOL_PACKS`, `PID_LAYERS` + `PROCESS-LINE-TEXT`, prompt rewrite, critique enum +6, benchmark v4, correctness 29), and the live COM smoke report verbatim. - [ ] **Step 5: Full gate** Run, in order, and fix anything red before committing: ``` uv run --frozen ruff check . && uv run --frozen ruff format --check . uv run --frozen pytest -x -q uv run --frozen python scripts/generate_tool_inventory.py --check uv run --frozen python scripts/check_doc_numbers.py uv run --frozen python benchmarks/compare_versions.py v1.5.1 ``` Expected: ruff clean; pytest green (every new test file included); inventory unchanged; doc numbers agree; A/B `29 / 29` with zero regressions. - [ ] **Step 6: Commit and mark the spec implemented** ```bash git add scripts/smoke_pid_com.py CLAUDE.md README.md CHANGELOG.md scripts/check_doc_numbers.py docs/superpowers/specs/2026-09-15-v1.6-pid-design.md git commit -m "docs(pid): track A shipped — CLAUDE.md, README, changelog with the live COM smoke record" ``` Track A is done when the gate in Step 5 is green and the roadmap's criteria 1 and 2 are demonstrated by `tests/test_pid_from_spec.py::test_finalize_scores_the_example_at_ninety_or_better` and `tests/test_pid_graph.py::test_foreign_blocks_classify_with_reduced_confidence_and_inferred_ports`.