From 4e523ac6ed7d5da5548c3991a40e3849d556a4c1 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Wed, 16 Sep 2026 10:49:30 +0300 Subject: [PATCH] feat(pid): instrument index, line list and equipment list from the graph Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- README.md | 2 +- discovery/aliases.py | 12 +++ docs/tool-inventory.json | 46 ++++++++- engineering/pid/deliverables.py | 175 ++++++++++++++++++++++++++++++++ server.py | 91 ++++++++++++++++- tests/test_pid_deliverables.py | 168 ++++++++++++++++++++++++++++++ tests/test_tool_registry.py | 11 +- 7 files changed, 497 insertions(+), 8 deletions(-) create mode 100644 engineering/pid/deliverables.py create mode 100644 tests/test_pid_deliverables.py diff --git a/README.md b/README.md index 4b80fda..72b2189 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ Live through COM on Windows, or headless through ezdxf anywhere — one typed co -> **v1.5 release snapshot:** 161 tools · 6 resources · 5 prompt templates · 1369 collected tests. +> **v1.5 release snapshot:** 164 tools · 6 resources · 5 prompt templates · 1369 collected tests. > 154 is the **registered** count; a default install advertises 149 over `tools/list`, > because `ENABLE_3D` is unset. `system_about` is the runtime authority. diff --git a/discovery/aliases.py b/discovery/aliases.py index 162c2c4..bbc8ba2 100644 --- a/discovery/aliases.py +++ b/discovery/aliases.py @@ -1396,6 +1396,18 @@ TOOL_ALIASES: dict[str, ToolAliases] = { "process graph", ), ), + "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"), + ), } diff --git a/docs/tool-inventory.json b/docs/tool-inventory.json index 3e4e34f..685dd20 100644 --- a/docs/tool-inventory.json +++ b/docs/tool-inventory.json @@ -3,7 +3,7 @@ "schema_version": 1, "package_version": "1.5.1", "totals": { - "tools": 161, + "tools": 164, "resources": 6, "prompts": 5, "groups": 20 @@ -21,7 +21,7 @@ "entity_query": 9, "layers": 14, "layouts": 12, - "pid": 4, + "pid": 7, "premium": 12, "solids": 5, "system": 7, @@ -1717,6 +1717,20 @@ "readOnlyHint": false } }, + { + "name": "pid_equipment_list", + "group": "pid", + "cost": "read", + "summary": "Equipment list derived from the P&ID graph (tag, kind, ports, connections); optional CSV.", + "tags": [ + "pid", + "query" + ], + "annotations": { + "title": "P&ID: Equipment List", + "readOnlyHint": true + } + }, { "name": "pid_graph", "group": "pid", @@ -1731,6 +1745,20 @@ "readOnlyHint": true } }, + { + "name": "pid_instrument_index", + "group": "pid", + "cost": "read", + "summary": "Instrument index derived from the P&ID graph (tag, function, loop, connections); optional CSV.", + "tags": [ + "pid", + "query" + ], + "annotations": { + "title": "P&ID: Instrument Index", + "readOnlyHint": true + } + }, { "name": "pid_line_draw", "group": "pid", @@ -1745,6 +1773,20 @@ "readOnlyHint": false } }, + { + "name": "pid_line_list", + "group": "pid", + "cost": "read", + "summary": "Line list derived from the P&ID graph (number, class, size, spec, from/to); optional CSV.", + "tags": [ + "pid", + "query" + ], + "annotations": { + "title": "P&ID: Line List", + "readOnlyHint": true + } + }, { "name": "pid_symbol_insert", "group": "pid", diff --git a/engineering/pid/deliverables.py b/engineering/pid/deliverables.py new file mode 100644 index 0000000..a1446b8 --- /dev/null +++ b/engineering/pid/deliverables.py @@ -0,0 +1,175 @@ +"""Instrument index, line list and equipment list — rows derived from the graph. + +Every row comes from ``build_graph`` (spec §9.5): the deliverables never read +the catalogue or a typed-in list, so a list of a foreign P&ID is as honest as +the graph's confidence says. Row keys are in the documented order; rows sort +by tag with a natural key (``P-2`` before ``P-10``) and untagged rows last. +""" + +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+)") + +EQUIPMENT_KINDS = ("equipment", "valve", "connector", "unknown_block") + + +def natural_key(tag): + """Sort key: digit runs compare numerically, ``None`` sorts after every 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]: + """``(tag, port)`` of an edge end: a node's tag and port, or the junction id.""" + 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 _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 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: list[str] = [] + 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"]) + 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 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_KINDS: + continue + ports = node["ports"] + 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: + """Write ``rows`` as CSV (header = row keys) inside ALLOWED_PATHS; return the resolved path.""" + 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: + """Build the graph once and derive ``which`` deliverable from it; optionally write CSV.""" + if which not in _BUILDERS: + raise ValueError(f"which must be one of {', '.join(_BUILDERS)}, got {which!r}") + 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"]} diff --git a/server.py b/server.py index 86e3059..f944226 100644 --- a/server.py +++ b/server.py @@ -6357,7 +6357,7 @@ async def solid_boolean( # --------------------------------------------------------------------------- -# ── SECTION 17: P&ID (4 tools) ────────────────────────────────────────────── +# ── SECTION 17: P&ID (7 tools) ────────────────────────────────────────────── # --------------------------------------------------------------------------- @@ -6580,6 +6580,95 @@ async def pid_graph( ) +@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, description="Endpoint-to-port snap distance (mm)") + ] = 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, description="Endpoint-to-port snap distance (mm)") + ] = 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. + + Rows come from `pid_graph`; untagged lines sort last with `line_number: null`. + 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), "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, description="Endpoint-to-port snap distance (mm)") + ] = 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. + + Rows come from `pid_graph`; the drawing is not modified and `csv_path` is + the only file this tool writes. + """ + from engineering.pid.deliverables import deliverable + + return await deliverable( + _backend(ctx), "equipment_list", csv_path, tolerance=tolerance, scope=scope + ) + + # --------------------------------------------------------------------------- # ── RESOURCES ─────────────────────────────────────────────────────────────── # --------------------------------------------------------------------------- diff --git a/tests/test_pid_deliverables.py b/tests/test_pid_deliverables.py new file mode 100644 index 0000000..7f5a28b --- /dev/null +++ b/tests/test_pid_deliverables.py @@ -0,0 +1,168 @@ +"""Instrument index, line list and equipment list are derived from the graph.""" + +from __future__ import annotations + +import csv + +import pytest +from fastmcp.exceptions import ToolError + +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", + ) + # A hand-drawn line off V-201's N4 nozzle: no XDATA, no label. The reader + # takes it as an edge (it ends on a port) with ``line_number: None`` — the + # untagged row spec 9.5 says sorts last. + n4 = vessel["ports"]["N4"] + await backend.entity_create_line( + n4["x"], n4["y"], n4["x"] + 40, n4["y"], layer="PROCESS-PIPING-MAIN" + ) + 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" + assert 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] * 4 + numbered = [r for r in rows if r["line_number"]] + # The signal line carries no size/service/spec, so its default-format + # number is the bare sequence "3" (empty fields drop out, spec 6.4) — and + # a natural sort files 3 before 100. + assert [r["line_number"] for r in numbered] == ["3", "100-P-1-CS1", "100-P-2-CS1"] + signal = numbered[0] + assert signal["line_class"] == "pneumatic" and signal["number_source"] == "xdata" + assert signal["from_tag"] == "FIC-101" and signal["to_tag"] == "FCV-101" + assert signal["to_port"] == "signal" + first = numbered[1] + assert first["from_tag"] == "P-101" and first["from_port"] == "discharge" + assert first["to_tag"] == "FCV-101" and first["length_mm"] > 0 + assert rows[-1]["line_number"] is None, "untagged rows sort last" + assert rows[-1]["from_tag"] == "V-201" and rows[-1]["from_port"] == "N4" + assert rows[-1]["to_tag"] is None and rows[-1]["number_source"] is None + + +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 + assert 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" + outside = tmp_path.parent / "outside.csv" + with pytest.raises(ToolError, match="not inside any allowed directory"): + await deliverable(backend, "line_list", csv_path=str(outside)) + assert not outside.exists(), "a refused path is never written" diff --git a/tests/test_tool_registry.py b/tests/test_tool_registry.py index 97a218b..708f0f1 100644 --- a/tests/test_tool_registry.py +++ b/tests/test_tool_registry.py @@ -509,8 +509,11 @@ async def test_tool_group_sizes_are_unchanged(): `entity_set_xdata` joined SECTION 5 (the getter is tagged `query`, the setter `modify`, so the pair splits across two groups). `pid` appeared (0 -> 2) when v1.6's SECTION 17 opened with `pid_symbol_list` / - `pid_symbol_insert`, then 2 -> 3 when `pid_line_draw` joined and 3 -> 4 - when the reader `pid_graph` joined; the `pid` tag is ranked first in `_GROUP_TAG_PRIORITY` so those tools file under + `pid_symbol_insert`, then 2 -> 3 when `pid_line_draw` joined, 3 -> 4 + when the reader `pid_graph` joined and 4 -> 7 when the three graph + deliverables (`pid_instrument_index` / `pid_line_list` / + `pid_equipment_list`) joined; the `pid` tag is ranked first in + `_GROUP_TAG_PRIORITY` so those tools file under `pid` rather than under their secondary `query` / `create` tags. Every other number here has been unchanged since the snapshot was taken. """ @@ -528,7 +531,7 @@ async def test_tool_group_sizes_are_unchanged(): "entity_query": 9, "layers": 14, "layouts": 12, - "pid": 4, + "pid": 7, "premium": 12, "solids": 5, "system": 7, @@ -537,4 +540,4 @@ async def test_tool_group_sizes_are_unchanged(): "validation": 1, "view": 4, } - assert sum(sizes.values()) == 161 + assert sum(sizes.values()) == 164