merge: track-e group H

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
Umut CAN
2026-09-22 11:10:17 +03:00
co-authored by Claude Opus 5
22 changed files with 4583 additions and 140 deletions
+66 -1
View File
@@ -33,7 +33,10 @@ _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)
try:
number = float(value)
except OverflowError as exc: # a 400-digit JSON integer is an int, not a float
raise TypeError(f"{where} must be finite") from exc
if not math.isfinite(number):
raise TypeError(f"{where} must be finite")
if positive and number <= 0:
@@ -204,6 +207,68 @@ def validate_attdef_specs(attdefs) -> list[dict]:
return out
def validate_base_point(base_x, base_y) -> tuple[float, float]:
"""``block_define``'s base point as finite floats; ``TypeError`` names the key.
Only ``float()`` was applied before, so ``float("nan")`` was written into
the BLOCK record headlessly and would reach AutoCAD as a NaN VARIANT.
"""
return (
_scalar(base_x, "block_define: 'base_x'"),
_scalar(base_y, "block_define: 'base_y'"),
)
def referenced_layers(entities: list[dict]) -> list[str]:
"""The distinct layers ``block_define``'s primitives reference, other than ``0``.
DXF and ActiveX layer tables are case-insensitive, so the set is keyed on
the folded name and keeps the first spelling seen: a request naming
``Alpha`` and ``ALPHA`` wants *one* layer. Deduping case-sensitively let
ezdxf write ``ALPHA`` and then raise ``DXFTableEntryError`` on ``Alpha``
mid-call, and let COM report two layers created for one ``Layers.Add``
that took effect.
"""
seen: dict[str, str] = {}
for spec in entities:
layer = spec["layer"]
if layer != "0" and layer.lower() not in seen:
seen[layer.lower()] = layer
return sorted(seen.values())
def validate_attribute_values(attributes, where: str = "attributes") -> dict[str, str]:
"""``{TAG: value}`` for ``block_insert`` / ``block_set_attributes`` as the
strings that will be written, refused before any write.
``str(value)`` used to be applied to whatever arrived, so a JSON ``null``
became the text ``None``, ``true`` became ``True`` and an object became its
Python repr — silently, on both engines. A string is taken as is (one line,
the DXF rule ``_one_line`` enforces); an int or a finite float is written
with ``str`` (``101`` → ``"101"``, ``2.5`` → ``"2.5"``); ``bool``, ``None``,
lists and objects are refused by tag. ``None`` for the whole mapping means
"no attributes".
"""
if attributes is None:
return {}
if not isinstance(attributes, dict):
raise TypeError(f"{where} must be an object of {{TAG: value}} pairs")
out: dict[str, str] = {}
for tag, value in attributes.items():
if not isinstance(tag, str) or not tag.strip():
raise TypeError(f"{where}: every tag must be a non-empty string")
label = f"{where}[{tag!r}]"
if isinstance(value, str):
out[tag] = _one_line(value, label)
elif isinstance(value, bool) or value is None or not isinstance(value, (int, float)):
raise TypeError(f"{label} must be a string or a number, got {type(value).__name__}")
else:
if isinstance(value, float) and not math.isfinite(value):
raise TypeError(f"{label} must be finite")
out[tag] = str(value)
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."""
+521 -14
View File
@@ -193,6 +193,70 @@ def _msp():
return doc.ModelSpace
def _int_member(obj, name: str) -> int | None:
"""``int(obj.<name>)`` or ``None`` when the member is absent or not a number."""
try:
return int(getattr(obj, name))
except Exception:
return None
def _owner_layout_block(doc, ent, handle):
"""The block table record that owns ``ent``, refused unless it is a layout.
``Explode()`` places the members in the owner space, so anything this
method adds alongside them must go there too — never ``ActiveLayout``,
which is whatever tab the user happens to have open. An owner that is a
block definition means ``ent`` is a nested reference; exploding it in place
would redefine the block under every other reference, so it is refused
before any call is dispatched. An owner that cannot be read at all is
refused too rather than guessed.
"""
try:
owner = doc.ObjectIdToObject(ent.OwnerID)
is_layout = bool(owner.IsLayout)
except Exception as exc:
raise RuntimeError(
f"Entity {handle} has no resolvable owner layout ({exc}); refusing to "
"explode into a guessed space"
) from exc
if not is_layout:
try:
name = str(owner.Name)
except Exception:
name = "?"
raise RuntimeError(
f"Entity {handle} is nested inside block definition {name!r}; explode the "
"outer reference instead (exploding here would redefine the block under "
"every other reference)"
)
return owner
def _require_block_defined(doc, name):
"""``doc.Blocks.Item(name)``, or a ``ValueError`` before ``InsertBlock`` runs.
``InsertBlock`` with an unknown name raises a bare COM error after the call
has already been dispatched; refusing here names the block, costs one
``Blocks.Item`` probe and matches the headless engine's message. A layout
block (``IsLayout``) cannot be inserted into itself — same refusal by name.
Shared by ``block_insert`` and ``entity_create_block_ref``.
"""
if not isinstance(name, str) or not name.strip():
raise TypeError("block name must be a non-empty string")
try:
block = doc.Blocks.Item(name)
except Exception as exc:
raise ValueError(f"block {name!r} is not defined") from exc
try:
is_layout = bool(block.IsLayout)
except Exception:
is_layout = False
if is_layout:
raise ValueError(f"block {name!r} is a layout block and cannot be inserted")
return block
_BUILTIN_LINETYPES = {"continuous", "bylayer", "byblock"}
@@ -794,6 +858,9 @@ class ComBackend(AutoCADBackend):
False, reason="window_capture_has_no_render_to_label"
),
"measure_area_acis": FeatureCapability(True, "native"),
"explode_opaque_members": FeatureCapability(
True, "native", reason="autocad_explode;ole_and_proxy_members_unverified_live"
),
"ocs_normalized": FeatureCapability(
True,
"activex_wcs",
@@ -2479,6 +2546,7 @@ class ComBackend(AutoCADBackend):
layer=None,
) -> EntityInfo:
def _sync():
_require_block_defined(_acad_doc(), name)
mspace = _msp()
ref = mspace.InsertBlock(
_apoint(x, y),
@@ -3247,7 +3315,12 @@ class ComBackend(AutoCADBackend):
attributes=None,
layer=None,
) -> EntityInfo:
from backends.block_specs import validate_attribute_values
values = validate_attribute_values(attributes)
def _sync():
_require_block_defined(_acad_doc(), name)
mspace = _msp()
ref = mspace.InsertBlock(
_apoint(x, y),
@@ -3259,25 +3332,392 @@ class ComBackend(AutoCADBackend):
)
if layer:
ref.Layer = layer
if attributes:
if values:
try:
attrs = ref.GetAttributes()
for attr in attrs:
tag = attr.TagString
if tag in attributes:
attr.TextString = str(attributes[tag])
if tag in values:
attr.TextString = values[tag]
except Exception as exc:
log.debug("Block insert: GetAttributes or attribute setting failed: %s", exc)
return _entity_info(ref)
return await self._run(_sync)
#: ActiveX twins of the headless ``_ATTRIB_TO_TEXT`` carry, written onto
#: the TEXT in this order and *before* ``Rotation``: ``Normal`` is the
#: frame ``Rotation`` is measured in, ``Thickness`` rides on the frame,
#: and the style members are frame-independent. ``Alignment`` /
#: ``TextAlignmentPoint`` (halign/valign + align_point) and ``Color`` are
#: handled apart because ActiveX orders them against the insertion point.
_ATTRIB_FRAME_MEMBERS = (
"Normal",
"Thickness",
"StyleName",
"ObliqueAngle",
"ScaleFactor",
"Backward",
"UpsideDown",
)
_ATTRIB_TEXT_MEMBERS = _ATTRIB_FRAME_MEMBERS + ("Alignment", "TextAlignmentPoint", "Color")
@classmethod
def _capture_attrib_text_members(cls, attr) -> dict:
"""Read the optional ATTRIB members a TEXT can take, skipping absent ones.
``Normal`` is a WCS unit vector and is only worth carrying when it is
not already +Z; ``TextAlignmentPoint`` is meaningless (and refused by
ActiveX on write) for a left-aligned text, so it is dropped when
``Alignment`` is ``acAlignmentLeft`` (0).
"""
out: dict = {}
for name in cls._ATTRIB_TEXT_MEMBERS:
try:
raw = getattr(attr, name)
except Exception:
continue
if name in ("Normal", "TextAlignmentPoint"):
try:
raw = tuple(float(v) for v in raw)
except (TypeError, ValueError):
continue
if name == "Normal" and ocs.is_wcs_frame(raw):
continue
elif name in ("Alignment", "Color"):
try:
raw = int(raw)
except (TypeError, ValueError):
continue
elif name in ("Backward", "UpsideDown"):
raw = bool(raw)
elif name == "StyleName":
raw = str(raw)
else:
try:
raw = float(raw)
except (TypeError, ValueError):
continue
out[name] = raw
if out.get("Alignment", 0) == 0:
out.pop("TextAlignmentPoint", None)
return out
#: ``AcadText.Alignment`` -> ``AcadMText.AttachmentPoint``, the inverse of
#: the map ezdxf applies when it embeds an MTEXT into an ATTRIB (the
#: attachment becomes ``halign``/``valign`` and both ``insert`` and
#: ``align_point`` become the MTEXT insert). Rows: Top 1-3, Middle 4-6,
#: Bottom 7-9; the baseline alignments (Left/Center/Right/Aligned/Middle/
#: Fit), which an embedded MTEXT never produces, fall to the nearest row.
_TEXT_ALIGNMENT_TO_MTEXT_ATTACHMENT = {
0: 7, # acAlignmentLeft -> BottomLeft
1: 8, # acAlignmentCenter -> BottomCenter
2: 9, # acAlignmentRight -> BottomRight
3: 7, # acAlignmentAligned -> BottomLeft
4: 5, # acAlignmentMiddle -> MiddleCenter
5: 7, # acAlignmentFit -> BottomLeft
6: 1, # acAlignmentTopLeft
7: 2, # acAlignmentTopCenter
8: 3, # acAlignmentTopRight
9: 4, # acAlignmentMiddleLeft
10: 5, # acAlignmentMiddleCenter
11: 6, # acAlignmentMiddleRight
12: 7, # acAlignmentBottomLeft
13: 8, # acAlignmentBottomCenter
14: 9, # acAlignmentBottomRight
}
@classmethod
def _capture_attrib(cls, attr) -> dict:
"""Everything a TEXT (or MTEXT) needs to stand in for ``attr`` (an
ATTRIB or ATTDEF).
A multi-line attribute (``MTextAttribute`` true) keeps its content in
``MTextAttributeContent`` -- ``TextString`` is one flattened string --
and its box width in ``MTextBoundaryWidth``; ``mtext`` marks it so the
writer builds an MTEXT. An object that does not expose *that* member
is single-line (the member is younger than the ActiveX surface, and a
``TextString`` fallback loses nothing).
``Invisible`` is *not* guessed: a read failure used to default to
visible, so a hidden attribute's value (a link, a cost code) became
visible text with ``ok: True``. The failure propagates, and the
caller of this helper reads it before anything is written.
"""
invisible = bool(attr.Invisible)
try:
is_mtext = bool(attr.MTextAttribute)
except Exception:
is_mtext = False
item = {
"text": str(attr.TextString),
"mtext": is_mtext,
"insertion": tuple(attr.InsertionPoint),
"height": float(attr.Height),
"rotation": float(attr.Rotation),
"layer": str(attr.Layer),
"invisible": invisible,
"members": cls._capture_attrib_text_members(attr),
}
if is_mtext:
try:
item["text"] = str(attr.MTextAttributeContent)
except Exception as exc:
log.debug("MTextAttributeContent unreadable, keeping TextString: %s", exc)
try:
item["width"] = float(attr.MTextBoundaryWidth)
except Exception:
item["width"] = 0.0 # unbounded box, as AddMText takes it
return item
@classmethod
def _add_mtext_from_capture(cls, owner, item: dict):
"""``owner.AddMText`` carrying a captured multi-line ATTRIB/ATTDEF.
The MTEXT's insertion point *is* its attachment anchor, so it is the
ATTRIB's ``TextAlignmentPoint`` when the attribute is box-aligned and
the ``InsertionPoint`` otherwise (ezdxf writes both from the embedded
MTEXT's insert, so they agree). Height, style and frame go on before
``Rotation``, then the attachment (the ATTRIB's ``Alignment`` mapped
through ``_TEXT_ALIGNMENT_TO_MTEXT_ATTACHMENT``), and the WCS anchor
is written *last*: ActiveX ``AttachmentPoint`` keeps the text body
where it is and relocates ``InsertionPoint`` to the new corner, so an
anchor written before it left every non-TopLeft attribute laid out
TopLeft-at-anchor with only the label moved (verified live on AutoCAD
2026: a BottomLeft two-line note landed two lines low; re-asserting
the anchor after the attachment moves the body). A frame change
(``Normal``) moves the OCS origin the same way, so the one late write
covers both. ``Thickness``, oblique, width factor and the generation
flags are TEXT-only members and stay behind.
"""
members = item["members"]
anchor = item["insertion"]
if members.get("Alignment", 0) != 0 and "TextAlignmentPoint" in members:
anchor = members["TextAlignmentPoint"]
point = _apoint(anchor[0], anchor[1], anchor[2] if len(anchor) > 2 else 0.0)
mtext = owner.AddMText(point, float(item.get("width", 0.0)), item["text"])
mtext.Height = item["height"]
if "StyleName" in members:
mtext.StyleName = members["StyleName"]
if "Normal" in members:
value = members["Normal"]
mtext.Normal = _apoint(value[0], value[1], value[2] if len(value) > 2 else 0.0)
mtext.Rotation = item["rotation"]
mtext.AttachmentPoint = cls._TEXT_ALIGNMENT_TO_MTEXT_ATTACHMENT.get(
members.get("Alignment", 0), 1
)
mtext.InsertionPoint = point # after the attachment and the frame: both relocate it
mtext.Layer = item["layer"]
if "Color" in members:
mtext.Color = members["Color"]
if item["invisible"]:
mtext.Visible = False
return mtext
@classmethod
def _add_text_from_capture(cls, owner, item: dict):
"""``owner.AddText`` carrying a captured ATTRIB/ATTDEF; returns the TEXT
(or, for a multi-line capture, the MTEXT ``_add_mtext_from_capture``
builds -- a TEXT of ``TextString`` would show ``\\P`` literally).
Every point or vector goes through ``_apoint`` (``VT_ARRAY|VT_R8``):
ActiveX refuses a plain Python tuple (marshalled ``VT_ARRAY|VT_VARIANT``)
with ``E_INVALIDARG``, and ``Normal`` is written after the explode and
the ATTDEF deletes, so a tuple there tore the drawing.
"""
if item.get("mtext"):
return cls._add_mtext_from_capture(owner, item)
ins = item["insertion"]
point = _apoint(ins[0], ins[1], ins[2] if len(ins) > 2 else 0.0)
text = owner.AddText(item["text"], point, item["height"])
members = item["members"]
for name in cls._ATTRIB_FRAME_MEMBERS:
if name not in members:
continue
value = members[name]
if name == "Normal":
value = _apoint(value[0], value[1], value[2] if len(value) > 2 else 0.0)
setattr(text, name, value)
text.Rotation = item["rotation"]
if "Normal" in members:
# The frame moved the OCS origin; the anchor was WCS.
text.InsertionPoint = point
if "Alignment" in members:
text.Alignment = members["Alignment"]
if "TextAlignmentPoint" in members:
tap = members["TextAlignmentPoint"]
text.TextAlignmentPoint = _apoint(tap[0], tap[1], tap[2] if len(tap) > 2 else 0.0)
text.Layer = item["layer"]
if "Color" in members:
text.Color = members["Color"]
if item["invisible"]:
text.Visible = False
return text
@staticmethod
def _undo_explode(exploded, texts) -> None:
"""Delete what a failed burst has already written (best effort).
An object the burst deleted itself (an ATTDEF placeholder) raises on a
second ``Delete``; that is skipped, every other failure is logged and
the next object is tried, so the original exception -- the one the
caller is about to re-raise -- stays the one the caller sees.
"""
for obj in tuple(texts) + tuple(exploded):
try:
obj.Delete()
except Exception as exc:
log.debug("explode rollback: %r not deleted: %s", obj, exc)
@staticmethod
def _is_constant_attdef(obj, constant_tags: set) -> bool:
"""Whether an exploded ``AcDbAttributeDefinition`` is a constant attribute.
``Constant`` is the documented member; when the object does not expose
it the tag is matched against ``GetConstantAttributes()`` read before
the explode, so a constant attribute is never mistaken for a value-less
placeholder and deleted.
"""
try:
return bool(obj.Constant)
except Exception:
pass
return str(obj.TagString) in constant_tags
async def block_explode(self, handle) -> dict:
"""Explode an INSERT through ActiveX; ATTRIB values survive as TEXT.
AutoCAD's EXPLODE keeps the attribute *definitions* (the tag names as
ATTDEF entities) and discards the values; ActiveX ``Explode()`` does
the same and, unlike the command, leaves the original reference in
place. This is the BURST rule instead: each ATTRIB's value, placement,
height, rotation, layer, frame and text style are read before the
explode, the ATTDEFs the explode returns are deleted, one ``AddText``
per ATTRIB carries the value (an invisible ATTRIB becomes an invisible
TEXT), and the reference itself is deleted. Unit-tested against a fake
ActiveX surface; ``Explode()``'s return shape is the ActiveX documented
one (an array of the new objects) and is exercised live by the
settings smoke.
A *constant* attribute has no ATTRIB: ``GetAttributes()`` excludes it
(``GetConstantAttributes()`` lists it) and ``Explode()`` hands it back
as an ``AcDbAttributeDefinition`` already at its WCS placement showing
its value. Deleting every ATTDEF therefore destroyed the constant text
with ``ok: True``; BURST converts it to TEXT, so a constant ATTDEF from
the explode is captured the same way an ATTRIB is, then replaced by a
TEXT, and only the value-less placeholders are just deleted.
An ATTRIB is an OCS entity and ``AddText`` builds a +Z TEXT, so the
frame is carried the way the headless engine carries ``extrusion`` /
``thickness``: ``Normal`` is written *before* ``Rotation`` (the
group-50 angle only means the same thing inside the same frame -- a
headless-authored mirrored reference's ATTRIB sits on ``(0, 0, -1)``;
AutoCAD's own MIRROR keeps +Z with ``XScaleFactor -1``) and the WCS
``InsertionPoint`` is re-asserted *after* it, because a frame change
moves the OCS origin under a point that was handed over in WCS. The
members ezdxf's ``_ATTRIB_TO_TEXT`` names have their ActiveX twins in
``_ATTRIB_TEXT_MEMBERS``; each is optional and skipped when the object
does not expose it, never written as a guess. ``Normal`` is written as
a ``VT_ARRAY|VT_R8`` VARIANT like every other point in this file; a
plain tuple is ``E_INVALIDARG`` live (verified on AutoCAD 2026).
A *multi-line* attribute (``MTextAttribute`` true) is captured from
``MTextAttributeContent`` and written back with ``AddMText`` -- its
``TextString`` is one flattened line and a TEXT of it would show the
``\\P`` breaks literally -- see ``_add_mtext_from_capture``.
The TEXTs go into the reference's *owner* block
(``ObjectIdToObject(OwnerID)``), which is where ``Explode()`` puts the
members; ``HandleToObject`` resolves a handle in any layout, so a title
block on a sheet exploded while Model was active used to get its tag
text in model space with ``ok: True``. Refused before the explode is
dispatched: a reference nested inside a block definition (owner
``IsLayout`` false), a MINSERT (``AcDbMInsertBlock``, which used to
fail the INSERT check with the misleading "not a block reference"),
and an xref (``Blocks.Item(Name).IsXRef``; ActiveX ``Explode()`` raises
on one, the headless engine used to delete it) -- the same three
refusals the headless engine names.
No read that decides what gets written is guessed: ``GetAttributes()``,
``GetConstantAttributes()`` and each attribute's ``Invisible`` are read
before ``Explode()`` and a failure propagates with nothing written
(a swallowed ``GetAttributes()`` used to burst *no* value and report
``ok: True``). A failure after the explode -- a constant ATTDEF whose
members cannot be read, an ``AddText`` that is refused -- undoes what
the explode added and the TEXTs written so far and leaves the
reference in place (``_undo_explode``), so the drawing is never left
half-burst under an exception.
"""
def _sync():
doc = _acad_doc()
ent = doc.HandleToObject(handle)
ent.Explode()
return {"ok": True, "exploded_handle": handle}
if ent.ObjectName == "AcDbMInsertBlock":
rows = _int_member(ent, "Rows")
cols = _int_member(ent, "Columns")
grid = f"{rows}x{cols} grid" if rows and cols else "grid"
raise RuntimeError(
f"Entity {handle} is a MINSERT ({grid} of {str(ent.Name)!r}); a "
"multi-insert cannot be exploded"
)
if ent.ObjectName != "AcDbBlockReference":
raise RuntimeError(f"Entity {handle} is not a block reference (INSERT)")
name = str(ent.Name)
try:
is_xref = bool(doc.Blocks.Item(name).IsXRef)
except Exception as exc:
log.debug("IsXRef probe failed for %r, treating as a local block: %s", name, exc)
is_xref = False
if is_xref:
raise RuntimeError(
f"Entity {handle} is an external reference {name!r}; an xref cannot "
"be exploded (bind it into the drawing first)"
)
owner = _owner_layout_block(doc, ent, handle)
# Every read that decides what the burst writes happens *before*
# ``Explode()`` and propagates: swallowing a failing
# ``GetAttributes()`` (a transient ``RPC_E_CALL_REJECTED`` while
# AutoCAD is busy is enough) went on to explode, delete every
# ATTDEF placeholder and the reference, and report ``ok: True``
# with ``attribute_texts: []`` -- every value destroyed silently,
# the defect this method exists to remove. Nothing has been
# written yet, so raising here costs nothing.
attrs = ent.GetAttributes()
captured = [self._capture_attrib(attr) for attr in attrs]
constant_tags: set = {str(attr.TagString) for attr in ent.GetConstantAttributes() or ()}
exploded = tuple(ent.Explode() or ())
inserted = []
attribute_texts = []
texts = []
try:
for obj in exploded:
if obj.ObjectName == "AcDbAttributeDefinition":
if self._is_constant_attdef(obj, constant_tags):
# A constant attribute's only text; already WCS here.
captured.append(self._capture_attrib(obj))
obj.Delete() # the tag placeholder EXPLODE leaves; the value is below
continue
inserted.append(str(obj.Handle))
for item in captured:
text = self._add_text_from_capture(owner, item)
texts.append(text)
attribute_texts.append(str(text.Handle))
except Exception:
# ActiveX ``Explode()`` leaves the reference in place, so the
# drawing is put back to exactly that: the members it added and
# the TEXTs written so far are removed, the reference stays,
# and the failure propagates instead of a half-burst symbol.
self._undo_explode(exploded, texts)
raise
ent.Delete()
_regen()
return {
"ok": True,
"exploded_handle": handle,
"inserted_handles": inserted,
"attribute_texts": attribute_texts,
"backend": "com",
}
return await self._run(_sync)
@@ -3294,6 +3734,10 @@ class ComBackend(AutoCADBackend):
return await self._run(_sync)
async def block_set_attributes(self, handle, attributes) -> dict:
from backends.block_specs import validate_attribute_values
values = validate_attribute_values(attributes)
def _sync():
doc = _acad_doc()
ref = doc.HandleToObject(handle)
@@ -3301,8 +3745,8 @@ class ComBackend(AutoCADBackend):
updated = []
for attr in attrs:
tag = attr.TagString
if tag in attributes:
attr.TextString = str(attributes[tag])
if tag in values:
attr.TextString = values[tag]
updated.append(tag)
return {"ok": True, "updated_tags": updated}
@@ -3436,18 +3880,25 @@ class ComBackend(AutoCADBackend):
base_x=0.0,
base_y=0.0,
overwrite=False,
create_layers=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``.
its INSERTs survive. The base point must be finite (a NaN would reach
AutoCAD as a VARIANT) and every primitive layer must exist
(``Layers.Item`` probe before ``Blocks.Add`` — ``obj.Layer = name`` for
an unknown layer would otherwise fail mid-loop with the definition half
written) unless ``create_layers`` adds them first. Unit-tested against
a fake ActiveX surface; executed live by ``scripts/smoke_pid_com.py``.
"""
from backends.block_specs import (
referenced_layers,
solid_vertices,
validate_attdef_specs,
validate_base_point,
validate_entity_specs,
)
@@ -3459,6 +3910,8 @@ class ComBackend(AutoCADBackend):
clean_name = sanitize_symbol_name(name, kind="block")
ents = validate_entity_specs(entities)
atts = validate_attdef_specs(attdefs or [])
base_xy = validate_base_point(base_x, base_y)
wanted_layers = referenced_layers(ents)
# acAlignmentLeft / Center / Right / MiddleCenter
align_map = {"left": 0, "center": 1, "right": 2, "middle_center": 10}
@@ -3479,7 +3932,23 @@ class ComBackend(AutoCADBackend):
"pass overwrite=true to replace its contents"
)
replaced = existing is not None
base = _apoint(float(base_x), float(base_y), 0.0)
missing_layers = []
# ``Layers.Item`` matches case-insensitively, like the table itself.
for layer in wanted_layers:
try:
doc.Layers.Item(layer)
except Exception:
missing_layers.append(layer)
if missing_layers and not create_layers:
raise ValueError(
"block_define: layer(s) "
+ ", ".join(repr(layer) for layer in missing_layers)
+ " do not exist in the drawing; create them with layer_create "
"or pass create_layers=true"
)
for layer in missing_layers:
doc.Layers.Add(layer)
base = _apoint(base_xy[0], base_xy[1], 0.0)
if existing is None:
block = doc.Blocks.Add(base, clean_name)
else:
@@ -3545,6 +4014,7 @@ class ComBackend(AutoCADBackend):
"entity_count": len(ents),
"attdef_count": len(atts),
"replaced": replaced,
"layers_created": missing_layers,
"backend": "com",
}
@@ -3898,6 +4368,22 @@ class ComBackend(AutoCADBackend):
def _sync():
doc = _acad_doc()
# `_UNDO _M` below is typed at the command line; with a command or
# prompt already active it would be swallowed as that command's
# input and no mark would exist for the rollback to go back to.
try:
cmd_active = int(doc.GetVariable("CMDACTIVE"))
except Exception as exc:
log.debug("CMDACTIVE read failed, proceeding anyway: %s", exc)
cmd_active = 0
if cmd_active:
return {
"ok": False,
"error": (
f"AutoCAD has an active command or prompt (CMDACTIVE={cmd_active}); "
"press ESC in AutoCAD before transaction_begin."
),
}
# R32: record the mark from inside the worker, immediately before
# setting it. `_transaction_active = True` used to run only after
# `_run` returned, so a timed-out begin left StartUndoMark open in
@@ -3906,10 +4392,22 @@ class ComBackend(AutoCADBackend):
# exact inverse of the R16 fix transaction_commit already carries.
# The abandoned STA thread cannot be cancelled, so the flag has to
# be written where the call happens; and it is written *before*
# StartUndoMark because a call that hangs inside it may still land.
# the marks because a call that hangs inside them may still land.
# A `_acad_doc()` that fails leaves the flag alone, which is right:
# nothing reached AutoCAD.
self._transaction_active = True
# Two marks, deliberately. StartUndoMark/EndUndoMark form an undo
# *group* (one UNDO step), not an UNDO Mark: on the live AutoCAD
# 2026 `_UNDO B` with no Mark answers "This will undo everything.
# OK? <Y>" and SendCommand blocks at that prompt until a human
# presses ESC (measured: rollback timed out at 60 s, the entities
# stayed, every COM client was rejected meanwhile). The Mark is set
# *before* the group so `_UNDO _B` in transaction_rollback lands
# exactly at the transaction's start — also when the group is
# empty, where `_UNDO 1` would be one step too many. Measured on
# AutoCAD 2026: Mark + group + 2 entities + `_UNDO _B` -> entities
# gone, CMDACTIVE 0, 47 ms; empty group -> nothing else undone.
doc.SendCommand("_.UNDO _M\n")
doc.StartUndoMark()
return {"ok": True, "message": "Transaction begun (AutoCAD undo mark set)"}
@@ -3929,12 +4427,21 @@ class ComBackend(AutoCADBackend):
self._transaction_active = False
async def transaction_rollback(self) -> dict:
# Without a transaction there is no Mark to go back to, and `_UNDO _B`
# would then prompt "This will undo everything. OK? <Y>" and block the
# STA worker at it (see transaction_begin). Refuse the same way the
# headless engine does.
if not self._transaction_active:
return {"ok": False, "error": "No active transaction to rollback"}
def _sync():
doc = _acad_doc()
doc.EndUndoMark()
# R16: route UNDO B through the CMDACTIVE-aware sender instead of a raw
# SendCommand that could deadlock if a command/prompt is active.
self._safe_send_command(doc, "_UNDO B")
# R16: route UNDO Back through the CMDACTIVE-aware sender instead of
# a raw SendCommand that could deadlock if a command/prompt is active.
# Back to the Mark transaction_begin set before the group, so this
# never prompts and never undoes anything older than the transaction.
self._safe_send_command(doc, "_.UNDO _B")
return {"ok": True, "message": "Transaction rolled back"}
try:
+12 -2
View File
@@ -58,9 +58,19 @@ class BlockContract(ABC):
base_x: float = 0.0,
base_y: float = 0.0,
overwrite: bool = False,
create_layers: 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}``."""
``backends/block_specs.py`` for the spec vocabulary.
Refusals, all before any write: a malformed spec (``TypeError`` naming
``entities[i]``/``attdefs[i]`` and the key), a non-finite or non-numeric
``base_x``/``base_y`` (``TypeError`` naming it), a name clash without
``overwrite`` (``ValueError``), and a primitive ``layer`` that does not
exist in the drawing (``ValueError`` listing the missing names) unless
``create_layers`` is true, in which case the missing layers are created
with the engine's defaults and listed in ``layers_created``. Returns
``{ok, name, entity_count, attdef_count, replaced, layers_created,
backend}``."""
...
+441 -31
View File
@@ -1027,6 +1027,13 @@ class EzdxfBackend(AutoCADBackend):
"measure_area_acis": FeatureCapability(
False, reason="acis_evaluation_requires_live_autocad"
),
"explode_opaque_members": FeatureCapability(
False,
reason=(
"ezdxf_cannot_transform:OLE2FRAME,VIEWPORT,"
"ACAD_PROXY_ENTITY_without_proxy_graphic;refused_before_writing"
),
),
"ocs_normalized": FeatureCapability(
True,
"wcs",
@@ -1087,6 +1094,28 @@ class EzdxfBackend(AutoCADBackend):
raise RuntimeError("No document open. Call drawing_new() or drawing_open() first.")
return self._doc
def _require_block(self, name):
"""The block definition ``name`` names, or a refusal before any write.
``add_blockref`` never checks the name and ``add_auto_attribs`` skips
autofill when ``Insert.block()`` resolves to None, so a typo'd name
used to yield a dangling INSERT that dropped its attribute values and
that ``doc.audit()`` later deletes. A layout block (``*Model_Space``,
``*Paper_Space``) is in ``doc.blocks`` too, but inserting one is the
reference cycle AutoCAD's INSERT refuses, so it is refused here by
name. Shared by ``block_insert`` and ``entity_create_block_ref`` so the
two tools that place a block reference agree on what a typo does.
"""
doc = self._require_doc()
if not isinstance(name, str) or not name.strip():
raise TypeError("block name must be a non-empty string")
if name not in doc.blocks:
raise ValueError(f"block {name!r} is not defined")
blk = doc.blocks.get(name)
if blk.block_record.is_any_layout:
raise ValueError(f"block {name!r} is a layout block and cannot be inserted")
return blk
def _msp(self):
"""The space new geometry goes into — model space, or the current layout.
@@ -2230,7 +2259,7 @@ class EzdxfBackend(AutoCADBackend):
frozen_text = self._format_measurement(entity.get_measurement())
entity.dxf.text = frozen_text
entity.transform(matrix)
self._transform(entity, matrix)
source.move_to_layout(entity, target)
# Where the entity now sits, expressed in paper coordinates.
@@ -3002,6 +3031,7 @@ class EzdxfBackend(AutoCADBackend):
) -> EntityInfo:
def _sync():
msp = self._msp()
self._require_block(name)
ent = msp.add_blockref(
name,
(float(x), float(y)),
@@ -3515,7 +3545,7 @@ class EzdxfBackend(AutoCADBackend):
async def entity_move(self, handle, dx, dy, dz=0.0) -> dict:
def _sync():
ent = self._get_entity(handle)
ent.translate(float(dx), float(dy), float(dz))
self._translate(ent, float(dx), float(dy), float(dz))
self._mark_dirty()
return {"ok": True, "handle": handle}
@@ -3526,7 +3556,7 @@ class EzdxfBackend(AutoCADBackend):
ent = self._get_entity(handle)
copy = ent.copy()
self._msp().add_entity(copy)
copy.translate(float(dx), float(dy), float(dz))
self._translate(copy, float(dx), float(dy), float(dz))
self._mark_dirty()
return _entity_info_dxf(copy)
@@ -3540,7 +3570,9 @@ class EzdxfBackend(AutoCADBackend):
m = Matrix44.z_rotate(math.radians(float(angle_deg)))
# Translate to origin, rotate, translate back
bx, by = float(base_x), float(base_y)
ent.transform(Matrix44.translate(-bx, -by, 0) @ m @ Matrix44.translate(bx, by, 0))
self._transform(
ent, Matrix44.translate(-bx, -by, 0) @ m @ Matrix44.translate(bx, by, 0)
)
self._mark_dirty()
return {"ok": True, "handle": handle}
@@ -3553,10 +3585,11 @@ class EzdxfBackend(AutoCADBackend):
s = float(factor)
bx, by = float(base_x), float(base_y)
ent.transform(
self._transform(
ent,
Matrix44.translate(-bx, -by, 0)
@ Matrix44.scale(s, s, s)
@ Matrix44.translate(bx, by, 0)
@ Matrix44.translate(bx, by, 0),
)
self._mark_dirty()
return {"ok": True, "handle": handle}
@@ -3607,7 +3640,9 @@ class EzdxfBackend(AutoCADBackend):
)
)
tx, ty = float(x1), float(y1)
copy.transform(Matrix44.translate(-tx, -ty, 0) @ m @ Matrix44.translate(tx, ty, 0))
self._transform(
copy, Matrix44.translate(-tx, -ty, 0) @ m @ Matrix44.translate(tx, ty, 0)
)
if delete_original:
self._msp().delete_entity(ent)
self._mark_dirty()
@@ -3714,7 +3749,7 @@ class EzdxfBackend(AutoCADBackend):
continue # skip original
copy = ent.copy()
msp.add_entity(copy)
copy.translate(c * float(col_spacing), r * float(row_spacing), 0)
self._translate(copy, c * float(col_spacing), r * float(row_spacing), 0)
results.append(_entity_info_dxf(copy))
self._mark_dirty()
return results
@@ -3750,7 +3785,7 @@ class EzdxfBackend(AutoCADBackend):
@ Matrix44.z_rotate(angle)
@ Matrix44.translate(cx, cy, 0)
)
copy.transform(m)
self._transform(copy, m)
results.append(_entity_info_dxf(copy))
self._mark_dirty()
return results
@@ -3928,13 +3963,27 @@ class EzdxfBackend(AutoCADBackend):
seen.add(key)
if layer and entity.dxf.get("layer", "0") != layer:
continue
current = entity.text if dxftype == "MTEXT" else entity.dxf.get("text", "")
# An ATTRIB/ATTDEF is read and written through the attribute
# helpers: a multi-line one keeps its content in an embedded
# MTEXT and ``dxf.text`` is only the first line, so matching
# and writing ``dxf.text`` alone reported ``replaced: 1`` while
# ``block_get_attributes`` and ``block_explode`` still carried
# the old value. One value, every surface -- the same rule
# ``block_set_attributes`` follows.
if dxftype == "MTEXT":
current = entity.text
elif dxftype in ("ATTRIB", "ATTDEF"):
current = self._attrib_value(entity)
else:
current = entity.dxf.get("text", "")
if not current or not pattern.search(current):
continue
updated = pattern.sub(replace, current)
if not dry_run:
if dxftype == "MTEXT":
entity.text = updated
elif dxftype in ("ATTRIB", "ATTDEF"):
self._write_attrib_value(entity, updated)
else:
entity.dxf.text = updated
changed.append(
@@ -4476,6 +4525,10 @@ class EzdxfBackend(AutoCADBackend):
attributes=None,
layer=None,
) -> EntityInfo:
from backends.block_specs import validate_attribute_values
values = validate_attribute_values(attributes)
def _sync():
msp = self._msp()
attribs: dict = {
@@ -4485,43 +4538,376 @@ class EzdxfBackend(AutoCADBackend):
}
if layer:
attribs["layer"] = layer
# `add_blockref` never checks the name, and `add_auto_attribs`
# silently skips autofill when `Insert.block()` resolves to None,
# so a typo'd name used to yield a dangling INSERT that dropped its
# attribute values and that `doc.audit()` later deletes. Refuse
# before writing, as COM's `InsertBlock` does for an unknown name.
if name not in self._doc.blocks:
raise ValueError(f"block {name!r} is not defined")
self._require_block(name)
ref = msp.add_blockref(name, (float(x), float(y)), dxfattribs=attribs)
if attributes:
if values:
# `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()})
ref.add_auto_attribs(values)
# A multi-line ATTDEF becomes a multi-line ATTRIB through
# `embed_mtext`, which mirrors only the first line of the value
# into `dxf.text`; the whole value goes there (the convention
# `_write_attrib_value` keeps) so an R2010 save keeps every line.
for attrib in ref.attribs:
if attrib.has_embedded_mtext_entity:
attrib.dxf.text = self._attrib_value(attrib)
self._mark_dirty()
return _entity_info_dxf(ref)
return await self._async(_sync)
@staticmethod
def _owner_layout_for_explode(ent, handle: str):
"""The layout that owns ``ent`` — model space or a paper-space layout.
Raises before any write when the owner is a block definition (a nested
reference) or cannot be resolved at all; guessing the current space
instead is how geometry ends up duplicated into the wrong tab.
"""
try:
layout = ent.get_layout()
except Exception as exc:
raise RuntimeError(
f"Entity {handle} has no resolvable owner layout ({exc}); refusing to "
"explode into a guessed space"
) from exc
if layout is None or not layout.is_any_layout:
owner = getattr(layout, "name", None) or ent.dxf.get("owner", "?")
raise RuntimeError(
f"Entity {handle} is nested inside block definition {owner!r}; explode "
"the outer reference instead (exploding here would redefine the block "
"under every other reference)"
)
return layout
#: ATTRIB DXF attributes carried onto the TEXT that replaces it on explode.
#: An ATTRIB is an OCS entity: ``insert``/``align_point`` are numbers in the
#: plane of its own ``extrusion``, so the frame travels with them. Without
#: it the TEXT took the OCS numbers with the default +Z frame, and a
#: mirrored reference (ezdxf reflects the ATTRIB to extrusion ``(0, 0, -1)``
#: while the INSERT keeps +Z with ``yscale=-1``) put the tag 50 units away
#: on the other side of the mirror axis — audit clean, ``plane_normal``
#: None, nothing said. Same numbers in the same frame is the same place.
_ATTRIB_TO_TEXT = (
"layer",
"color",
"style",
"height",
"rotation",
"width",
"oblique",
"halign",
"valign",
"text_generation_flag",
"insert",
"align_point",
"extrusion",
"thickness",
)
@classmethod
def _text_attribs_of(cls, attrib) -> dict:
"""The ``_ATTRIB_TO_TEXT`` members ``attrib`` (ATTRIB or ATTDEF) carries."""
return {key: attrib.dxf.get(key) for key in cls._ATTRIB_TO_TEXT if attrib.dxf.hasattr(key)}
@staticmethod
def _anchored_mtext(attrib):
"""The embedded MTEXT of a multi-line ``attrib`` (ATTRIB or ATTDEF) as a
virtual entity placed where the attribute itself is.
A multi-line attribute keeps its placement on two surfaces: the
ATTRIB's own ``insert``/``align_point`` (OCS, the frame of its
``extrusion``) and the embedded MTEXT's ``insert`` (WCS). ezdxf keeps
them in step through ``transform`` (``BaseAttrib`` overrides it) but
not through ``translate``: ``Insert.translate`` calls ``Text.translate``
on every ATTRIB, which moves the ATTRIB's two points and nothing else
(ezdxf 1.4.4 ``attrib.py`` has no ``translate``). So after a move the
virtual MTEXT still said where the attribute *was*, ``block_explode``
burst the note 50 units from its symbol with a clean audit, and a
value write (``set_mtext`` re-places the ATTRIB from the MTEXT) pulled
the attribute back to the old spot while the INSERT stayed put.
The ATTRIB's placement is the attribute's placement -- it is what
``entity_get`` reports, what a single-line attribute has, and what
every ezdxf write path maintains -- so the virtual entity is anchored
to it: the inverse of ezdxf's own ``_update_location_from_mtext``
(``get_placement()`` picks ``insert`` for a LEFT-aligned text and
``align_point`` otherwise, the same rule the renderer uses).
"""
mtext = attrib.virtual_mtext_entity()
mtext.dxf.insert = attrib.ocs().to_wcs(attrib.get_placement()[1])
return mtext
@classmethod
def _settle_attrib_mtext(cls, attrib) -> None:
"""Put the embedded MTEXT of ``attrib`` where the ATTRIB now is; a
no-op for a single-line attribute.
Runs after every ``translate`` of an INSERT (``entity_move``,
``entity_copy``, ``entity_array_rectangular``). The round trip is the
one ezdxf's ``BaseAttrib.transform`` uses; ``set_mtext`` also mirrors
only the first line into ``dxf.text``, so the value (the whole
``Line1\\PLine2`` -- see ``_write_attrib_value``) is put back.
"""
if not attrib.has_embedded_mtext_entity:
return
text = attrib.dxf.text
attrib.set_mtext(cls._anchored_mtext(attrib), graphic_properties=False)
attrib.dxf.text = text
@classmethod
def _translate(cls, ent, dx: float, dy: float, dz: float) -> None:
"""``ent.translate(dx, dy, dz)`` that carries a multi-line ATTRIB's
embedded MTEXT along (see ``_anchored_mtext``)."""
ent.translate(dx, dy, dz)
if ent.dxftype() == "INSERT":
for attrib in ent.attribs:
cls._settle_attrib_mtext(attrib)
@staticmethod
def _transform(ent, m) -> None:
"""``ent.transform(m)`` that keeps group 1 of a multi-line ATTRIB whole.
ezdxf's ``BaseAttrib.transform`` rebuilds the ATTRIB through
``set_mtext``, which mirrors only the *first line* into ``dxf.text``:
a rotate, scale or mirror after ``block_set_attributes`` turned
``A\\PB`` back into ``A`` on the one surface an R2010 save keeps, and
the reloaded drawing had lost every line but the first. The content
does not change under a transform, so the value is put back.
"""
if ent.dxftype() != "INSERT":
ent.transform(m)
return
texts = [(a, a.dxf.text) for a in ent.attribs if a.has_embedded_mtext_entity]
ent.transform(m)
for attrib, text in texts:
attrib.dxf.text = text
@classmethod
def _burst_attrib(cls, layout, attrib, matrix=None):
"""Add the entity that stands in for ``attrib`` (ATTRIB or ATTDEF) to
``layout`` and return it: a TEXT carrying the value, or -- for a
multi-line attribute -- an MTEXT carrying every line.
A multi-line ATTRIB keeps its content in an embedded MTEXT; its
``dxf.text`` is only the first line (ezdxf) or ``Line1\\PLine2...``
(AutoCAD), so a TEXT built from it drops the other lines or renders
literal ``\\P`` -- with a clean audit. BURST emits an MTEXT there.
``_anchored_mtext`` carries the ATTRIB's graphic properties and its
placement (the embedded MTEXT's own ``insert`` is stale after a
``translate``, see there). ``matrix``, when given, is the reference's
matrix, for an ATTDEF still in block coordinates.
"""
if attrib.has_embedded_mtext_entity:
entity = cls._anchored_mtext(attrib)
if matrix is not None:
entity.transform(matrix)
layout.add_entity(entity)
else:
entity = layout.add_text(attrib.dxf.text, dxfattribs=cls._text_attribs_of(attrib))
if matrix is not None:
entity.transform(matrix)
if attrib.is_invisible:
entity.dxf.invisible = 1
return entity
@staticmethod
def _refuse_unexplodable_reference(ent, handle: str) -> None:
"""Raise before any write for the two INSERT kinds ``virtual_entities()``
cannot explode -- both used to be destroyed with ``ok: True``.
An *xref* INSERT yields no virtual entities (the geometry is in the
other file), so the reference was deleted and nothing put in its
place; AutoCAD refuses to explode an xref (bind it first). A
*MINSERT* (``mcount`` > 1) yields only its first cell -- ``multi_insert()``
resolves the grid -- so a 2x3 grid exploded into one cell and five
vanished; AutoCAD refuses to explode a MINSERT.
"""
name = ent.dxf.get("name", "?")
block = ent.block()
record = getattr(block, "block_record", None)
if record is not None and record.is_xref: # covers XREF_OVERLAY too
raise RuntimeError(
f"Entity {handle} is an external reference {name!r}; an xref cannot "
"be exploded (bind it into the drawing first)"
)
if ent.mcount > 1:
rows = ent.dxf.get("row_count", 1)
cols = ent.dxf.get("column_count", 1)
raise RuntimeError(
f"Entity {handle} is a MINSERT ({rows}x{cols} grid of {name!r}); a "
"multi-insert cannot be exploded"
)
@staticmethod
def _explodable_members(ent, handle: str) -> list:
"""The reference's members as virtual entities, or a typed refusal
before any write when ezdxf would drop one.
``virtual_entities()`` skips every member it cannot copy (OLE2FRAME)
or transform (VIEWPORT) with a debug-level log only, and an
ACAD_PROXY_ENTITY contributes its proxy graphic -- nothing at all when
it carries none -- so a title block's OLE logo or a vertical product's
proxy geometry used to vanish with ``ok: True`` and a clean audit. The
list is materialized here so the skips are known before the layout is
touched; AutoCAD explodes all of these natively.
"""
skipped: list[str] = []
def _collect(entity, reason):
entity_handle = entity.dxf.get("handle") or "?"
skipped.append(f"{entity.dxftype()} {entity_handle} ({reason})")
members = list(ent.virtual_entities(skipped_entity_callback=_collect))
block = ent.block()
for member in block if block is not None else ():
if member.dxftype() != "ACAD_PROXY_ENTITY":
continue
if not any(True for _ in member.virtual_entities()):
skipped.append(f"ACAD_PROXY_ENTITY {member.dxf.handle} (no proxy graphic)")
if skipped:
name = ent.dxf.get("name", "?")
raise UnsupportedCapabilityError(
"explode_opaque_members",
f"block_explode: entity {handle} references block {name!r} whose "
f"members {', '.join(skipped)} cannot be exploded by the headless "
"ezdxf backend (it cannot transform them; exploding would delete "
"them). Nothing was written. Switch to the live COM backend "
"(AUTOCAD_MCP_BACKEND=com, needs Windows + AutoCAD), which hands "
"the explode to AutoCAD.",
)
return members
async def block_explode(self, handle) -> dict:
"""Explode an INSERT into its members; ATTRIB values survive as TEXT.
``virtual_entities()`` yields the definition's geometry only — no
ATTDEF, no ATTRIB — so an exploded tagged symbol used to lose its tag
text silently. This is the BURST rule (Express Tools): every attached
ATTRIB becomes a TEXT at the same WCS placement, height, rotation,
style and layer, carrying the *value*; an invisible ATTRIB becomes an
invisible TEXT rather than appearing. The new handles are reported as
``attribute_texts``, separate from the geometry's ``inserted_handles``.
A *constant* attribute (ATTDEF flag 2) carries its value in the
definition and, in an AutoCAD-authored file, has no ATTRIB on the
reference — ``virtual_entities()`` skips ATTDEF, so its visible text
used to vanish with a clean audit. BURST converts it to TEXT, so every
constant ATTDEF whose tag has no attached ATTRIB is copied as a TEXT
and transformed by the reference's matrix — the same path
``add_auto_attribs`` takes, so it lands where the ATTRIB the headless
``block_insert`` attaches would have (that ATTRIB, when present, is
converted above and the definition is not converted twice).
A *multi-line* attribute (ATTRIB or constant ATTDEF with an embedded
MTEXT) becomes an MTEXT carrying every line, as BURST does -- its
``dxf.text`` holds only the first line (ezdxf) or the lines joined
with literal ``\\P`` (AutoCAD), so a TEXT silently lost content. See
``_burst_attrib``.
The members and the TEXTs go into the INSERT's *owner* layout, not the
current one: ``_get_entity`` resolves a handle in any layout, so a title
block on a sheet exploded while Model was current used to have its
geometry copied into model space and then die on ``delete_entity``
with the reference still alive on the sheet. Refused before anything
is written: a reference nested inside a block definition (exploding
it there would redefine the block under every other reference), an
xref, and a MINSERT -- see ``_refuse_unexplodable_reference`` for the
last two, which ``virtual_entities()`` cannot represent and which used
to be deleted with ``ok: True`` -- and a reference whose definition
holds a member ezdxf cannot copy or transform (OLE2FRAME, VIEWPORT, a
proxy entity without proxy graphics), refused by member with the
capability key ``explode_opaque_members`` -- see
``_explodable_members``; those used to be dropped from the result with
``ok: True`` while the reference was deleted.
"""
def _sync():
ent = self._get_entity(handle)
if ent.dxftype() != "INSERT":
raise RuntimeError(f"Entity {handle} is not a block reference (INSERT)")
msp = self._msp()
# Decompose: add individual entities to modelspace
self._refuse_unexplodable_reference(ent, handle)
msp = self._owner_layout_for_explode(ent, handle)
members = self._explodable_members(ent, handle)
inserted = []
for sub in ent.virtual_entities():
for sub in members:
sub_copy = sub.copy()
msp.add_entity(sub_copy)
inserted.append(sub_copy.dxf.handle)
attribute_texts = []
attached_tags = set()
for attrib in ent.attribs:
attached_tags.add(attrib.dxf.tag)
attribute_texts.append(self._burst_attrib(msp, attrib).dxf.handle)
block = ent.block()
attdefs = block.attdefs() if block is not None else ()
matrix = ent.matrix44()
for attdef in attdefs:
if not attdef.is_const or attdef.dxf.get("tag") in attached_tags:
continue
if not attdef.dxf.hasattr("insert"):
continue # a structure error; nowhere to place it
attribute_texts.append(self._burst_attrib(msp, attdef, matrix).dxf.handle)
msp.delete_entity(ent)
self._mark_dirty()
return {"ok": True, "inserted_handles": inserted}
return {
"ok": True,
"exploded_handle": handle,
"inserted_handles": inserted,
"attribute_texts": attribute_texts,
"backend": "ezdxf",
}
return await self._async(_sync)
@staticmethod
def _attrib_value(attrib) -> str:
"""The value an ATTRIB carries, as AutoCAD's ``TextString`` reports it.
A multi-line attribute keeps its content in an embedded MTEXT and its
``dxf.text`` is only the first line when ezdxf's ``embed_mtext`` /
``set_mtext`` authored it (AutoCAD, and ``_write_attrib_value``, write
``Line1\\PLine2``), so the embedded content is the value; a
single-line attribute's value is ``dxf.text``.
"""
if attrib.has_embedded_mtext_entity:
return attrib.virtual_mtext_entity().text
return attrib.dxf.text
@classmethod
def _write_attrib_value(cls, attrib, value: str) -> None:
"""Write ``value`` into ``attrib`` on every surface it has.
Setting only ``dxf.text`` on a multi-line ATTRIB left the embedded
MTEXT at its old content: ``block_get_attributes`` reported the new
value, ``block_explode`` burst the old one, and on an R2010 document
(no embedded MTEXT exported) a save/reload carried the new value the
exploded drawing never showed. The embedded MTEXT is rebuilt from its
virtual entity with the new content -- the round trip ezdxf's
``BaseAttrib.transform`` itself uses -- so the MTEXT and the placement
stay one attribute. The virtual entity is anchored to the ATTRIB's
own placement first (``_anchored_mtext``): ``set_mtext`` re-places
the ATTRIB from the MTEXT, and after an ``entity_move`` the embedded
MTEXT still sat where the attribute *was*, so a value edit teleported
the attribute 50 units away from its symbol.
``set_mtext`` then mirrors only the *first line* into ``dxf.text``
(ezdxf's own choice), and below R2018 ezdxf exports no embedded MTEXT,
so the default (R2010) document saved ``NEW ONE`` where the reader had
promised ``NEW ONE\\PNEW TWO``. ``dxf.text`` is therefore written last
with the whole value: that is AutoCAD's own convention (``TextString``
of a live multi-line attribute reads ``'LINE ONE\\PLINE TWO'``), the
reader prefers the embedded MTEXT when there is one, and the file
carries every line on every DXF version.
"""
if attrib.has_embedded_mtext_entity:
mtext = cls._anchored_mtext(attrib)
mtext.text = value
attrib.set_mtext(mtext, graphic_properties=False)
attrib.dxf.text = value
async def block_get_attributes(self, handle) -> dict:
def _sync():
ent = self._get_entity(handle)
@@ -4529,12 +4915,16 @@ class EzdxfBackend(AutoCADBackend):
raise RuntimeError(f"Entity {handle} is not a block reference")
result = {}
for attrib in ent.attribs:
result[attrib.dxf.tag] = attrib.dxf.text
result[attrib.dxf.tag] = self._attrib_value(attrib)
return result
return await self._async(_sync)
async def block_set_attributes(self, handle, attributes) -> dict:
from backends.block_specs import validate_attribute_values
values = validate_attribute_values(attributes)
def _sync():
ent = self._get_entity(handle)
if ent.dxftype() != "INSERT":
@@ -4542,8 +4932,8 @@ class EzdxfBackend(AutoCADBackend):
updated = []
for attrib in ent.attribs:
tag = attrib.dxf.tag
if tag in attributes:
attrib.dxf.text = str(attributes[tag])
if tag in values:
self._write_attrib_value(attrib, values[tag])
updated.append(tag)
self._mark_dirty()
return {"ok": True, "updated_tags": updated}
@@ -4675,20 +5065,27 @@ class EzdxfBackend(AutoCADBackend):
base_x=0.0,
base_y=0.0,
overwrite=False,
create_layers=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).
definition behind: specs, the base point (``base_x``/``base_y`` must be
finite numbers — ``float("nan")`` used to be written into the BLOCK
record) and the layer table (a primitive's ``layer`` must exist, or
``create_layers`` must be true — a dangling layer reference passes
``doc.audit()`` silently). ``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 (
referenced_layers,
solid_vertices,
validate_attdef_specs,
validate_base_point,
validate_entity_specs,
)
from security import sanitize_symbol_name
@@ -4701,6 +5098,8 @@ class EzdxfBackend(AutoCADBackend):
clean_name = sanitize_symbol_name(name, kind="block")
ents = validate_entity_specs(entities)
atts = validate_attdef_specs(attdefs or [])
base = (*validate_base_point(base_x, base_y), 0.0)
wanted_layers = referenced_layers(ents)
align_map = {
"left": TextEntityAlignment.LEFT,
"center": TextEntityAlignment.CENTER,
@@ -4717,7 +5116,17 @@ class EzdxfBackend(AutoCADBackend):
"pass overwrite=true to replace its contents"
)
replaced = existing is not None
base = (float(base_x), float(base_y), 0.0)
# ``in doc.layers`` is case-insensitive, like the table itself.
missing_layers = [layer for layer in wanted_layers if layer not in doc.layers]
if missing_layers and not create_layers:
raise ValueError(
"block_define: layer(s) "
+ ", ".join(repr(layer) for layer in missing_layers)
+ " do not exist in the drawing; create them with layer_create "
"or pass create_layers=true"
)
for layer in missing_layers:
doc.layers.add(layer)
if existing is None:
blk = doc.blocks.new(name=clean_name, base_point=base)
else:
@@ -4784,6 +5193,7 @@ class EzdxfBackend(AutoCADBackend):
"entity_count": len(ents),
"attdef_count": len(atts),
"replaced": replaced,
"layers_created": missing_layers,
"backend": "ezdxf",
}
+15 -7
View File
@@ -381,19 +381,27 @@ class AutoCADMCPProAdapter(BenchmarkAdapter):
graph = result["graph"]
index = instrument_index(graph)
lines = line_list(graph)
# The spec numbers its two process lines; the signal line and the
# connector line are left to the default ``number_format``, which with
# no size/service/spec/insulation collapses to the bare sequence
# number. Both must come back through XDATA, not a label search.
# The spec numbers its two process lines; the connector line is left
# to the default ``number_format``, which with no size/service/spec/
# insulation collapses to the bare sequence number, and it must come
# back through XDATA, not a label search. The electric signal line is
# unnumbered by rule (ISA-5.1 signal lines carry no pipe line number)
# and must not have consumed a sequence number: the connector is "3".
authored = "100-P-1001-CS1"
numbers = {(r["from_tag"], r["to_tag"]): r["line_number"] for r in lines}
authored_ok = (
numbers.get(("P-101", "FCV-101")) == authored
and numbers.get(("FCV-101", "V-201")) == authored
)
sequenced = [r for r in lines if r["line_number"] != authored]
sequenced_ok = len(sequenced) == 2 and all(
r["line_number"].isdigit() and r["number_source"] == "xdata" for r in sequenced
others = [r for r in lines if r["line_number"] != authored]
signal = [r for r in others if r["line_class"] == "electric"]
sequenced = [r for r in others if r["line_class"] != "electric"]
sequenced_ok = (
len(signal) == 1
and signal[0]["line_number"] is None
and len(sequenced) == 1
and sequenced[0]["line_number"] == "3"
and sequenced[0]["number_source"] == "xdata"
)
passed = (
len(graph["nodes"]) == 5
+5 -1
View File
@@ -220,13 +220,15 @@ TOOL_ALIASES: dict[str, ToolAliases] = {
),
),
"block_explode": ToolAliases(
acad=("EXPLODE",),
acad=("EXPLODE", "BURST"),
synonyms=(
"explode",
"break a block apart",
"ungroup",
"split a block into entities",
"flatten a symbol",
"burst a block",
"explode and keep the attribute text",
),
),
"block_find_references": ToolAliases(
@@ -1382,6 +1384,8 @@ TOOL_ALIASES: dict[str, ToolAliases] = {
"line number",
"connect the pump to the vessel",
"pneumatic signal",
"unnumbered signal line",
"route a line through waypoints",
),
),
"pid_graph": ToolAliases(
+292
View File
@@ -0,0 +1,292 @@
# Track A (v1.6 P&ID) — review backlog
**Source:** the plan-compliance and adversarial review rounds of Track A (2026-09-15/16),
waves 1–4. Blocking findings were fixed inside Track A (`fix(task-N)` commits) and are
not listed. Every *minor* finding is here, de-duplicated, with one disposition:
- **fixed-in-task-N** — Track E group H (`docs/superpowers/plans/2026-09-16-v1.6-settings-implementation.md`, Tasks 1–7)
- **fixed-in-track-A** — resolved on main by a later Track A commit (evidence cited)
- **accepted-limitation** — kept as is, with the reason
- **deferred-1.7** — a real item for the 1.7 roadmap
Reading key: `W1-T1-03` = wave 1, Track A Task 1, finding 3. Files are repository-relative.
## Wave 1
#### Track A Task 1 — `block_insert(attributes=…)` (ezdxf)
| ID | File | Finding | Disposition |
|---|---|---|---|
| W1-T1-01 | backends/ezdxf_backend.py | The undefined-block gate (`6f4bda5`) was added without being listed as a deviation. | accepted-limitation (reporting gap; the gate is kept and shared by Task 1) |
| W1-T1-02 | plan | Branch forked before the docs commits; no code diverged. | accepted-limitation (process; merged clean) |
| W1-T1-03 | backends/ezdxf_backend.py | `attributes` was coerced after `add_blockref`, so a non-mapping failed after the write and skipped `_mark_dirty`. | fixed-in-task-3 |
| W1-T1-04 | backends/ezdxf_backend.py | `str(v)` wrote Python reprs (`None`, `True`, a dict) into ATTRIBs on both engines. | fixed-in-task-3 |
| W1-T1-05 | backends/ezdxf_backend.py | Tag matching is exact-case; `{"tag": …}` silently drops against ATTDEF `TAG`. | deferred-1.7 (normalise keys to upper case; the server's own ATTDEF tags are upper-case by `ATTDEF_TAG_RE`) |
| W1-T1-06 | backends/ezdxf_backend.py | `*Model_Space` / `*Paper_Space` pass the existence check and insert a reference cycle. | fixed-in-task-1 |
| W1-T1-07 | backends/ezdxf_backend.py | `entity_create_block_ref` still accepts an undefined block; `doc.audit()` deletes the INSERT later. | fixed-in-task-1 |
| W1-T1-08 | tests/test_block_insert_attributes.py | `attributes=None` yields no ATTRIBs headlessly while COM's `InsertBlock` instantiates ATTDEF defaults; refusal types diverged (ValueError vs RuntimeError). | refusal type: fixed-in-task-1 (COM raises the same ValueError); default ATTRIBs: deferred-1.7 (engine divergence on ATTDEF defaults) |
| W1-T1-09 | backends/ezdxf_backend.py | `block_explode` drops the ATTRIB text. | fixed-in-task-2 |
#### Track A Task 2 — `backends/block_specs.py`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W1-T2-01 | security.py | `illegal_symbol_name_chars` added outside the task's file list, unreported. | accepted-limitation (reporting gap; harmless, tested) |
| W1-T2-02 | backends/block_specs.py | `fullmatch`, layer-character gate and `_one_line` are stricter than the plan stated. | accepted-limitation (all serve the validate-before-write rule; pinned by tests) |
| W1-T2-03 | plan / spec | Spec §4.7 per-entity `color`/`linetype`/`lineweight` overrides were dropped by the plan; unknown keys are silently ignored. | deferred-1.7 (carry the three keys through or refuse unknown keys by index) |
| W1-T2-04 | backends/block_specs.py | `_scalar` let `10**400` escape as `OverflowError` instead of the named `TypeError`. | fixed-in-task-4 |
| W1-T2-05 | backends/block_specs.py | A non-list `entities` (dict, str, generator) is refused as "empty". | deferred-1.7 |
| W1-T2-06 | backends/block_specs.py | `_layer` gates characters but not the 255-char length rule. | deferred-1.7 |
| W1-T2-07 | backends/block_specs.py | Refusal messages echo the caller's value unbounded (`got {value!r}`; measured 2 MB messages). | deferred-1.7 (truncate with reprlib) |
| W1-T2-08 | backends/block_specs.py | Same as W1-T2-03 (unknown keys such as `rotation` silently dropped). | deferred-1.7 (with W1-T2-03) |
#### Track A Task 6 — ISA-5.1 tag grammar
| ID | File | Finding | Disposition |
|---|---|---|---|
| W1-T6-01 | engineering/pid/tags.py | Three grammar changes (X user-defined, no output after V, O/C only after ZS) unreported. | accepted-limitation (the fixture is the contract; documented in `describe_tables()['rules']`) |
| W1-T6-02 | engineering/pid/tags.py | Two of those go past the spec §8 wording. | accepted-limitation (spec text to be reconciled by the 1.7 docs pass) |
| W1-T6-03 | spec | Spec example meanings (`Flow rate`/`Indicate`/`Control`) differ from the tables (`Flow`/`Indicator`/`Controller`). | accepted-limitation (keys match; consumers pin the tables) |
| W1-T6-04 | engineering/pid/tags.py | The "unexpected letter" error names the first trailing letter, not the offender (`LAHX-1` → reports H). | deferred-1.7 |
| W1-T6-05 | engineering/pid/tags.py | `equipment_prefixes={}` falls back to the default map (`or` instead of `is None`); non-string values pass through. | deferred-1.7 (also W4-T19-05) |
| W1-T6-06 | tests/data/isa51_tags.json | No fixture row for K (rate of change / control station), M trailing, O readout, B/N/X user-defined readouts. | deferred-1.7 (≈5 rows) |
| W1-T6-07 | engineering/pid/tags.py | O/C trailing narrowed to Z-first tags beyond the spec text. | accepted-limitation (documented resolution of a spec/fixture conflict) |
#### Track A Task 12 — `engineering/pid/lines.py`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W1-T12-01 | engineering/pid/lines.py | `axis_direction`, the `string.Formatter` parser and finiteness checks were added unreported. | accepted-limitation (reporting gap; pinned by tests) |
| W1-T12-02 | engineering/pid/__init__.py | Created outside the task's file list (branch predated Task 5). | accepted-limitation (byte-identical to Task 5's) |
| W1-T12-03 | engineering/pid/lines.py | `direct` and waypoint modes accept a negative / NaN `stub`. | deferred-1.7 (hoist the stub check above the mode branch) |
| W1-T12-04 | engineering/pid/lines.py | A malformed waypoint (`{"x":…}`, a flat list) escapes as `KeyError` / `TypeError` instead of `ValueError` naming `waypoints[i]`. | deferred-1.7 (also W3-T13-11) |
| W1-T12-05 | engineering/pid/lines.py | `count_crossings` undercounts a crossing that lands exactly on the other polyline's vertex. | deferred-1.7 (reported metric; orthogonal auto routes cannot produce it) |
| W1-T12-06 | engineering/pid/lines.py | A format spec containing `-` (`{seq:-05d}`) is refused with a misleading message. | accepted-limitation (a refusal, not a wrong number; hyphen inside a field spec is unsupported) |
| W1-T12-07 | engineering/pid/lines.py | `label_placement([(0,0)])` raises an unnamed error; a closed-loop waypoint route is accepted; `{{seq}}` is copied through. | accepted-limitation (unreachable through the tools; cosmetic) |
## Wave 2
#### Track A Task 3 — `block_define`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W2-T3-01 | README.md | Snapshot bumped while lines 23/30/106 still said 154. | fixed-in-track-A (Task 21 README rewrite: "166 tools in 20 groups") |
| W2-T3-02 | tests/test_tool_registry.py | Frozen snapshot edited outside the file list. | accepted-limitation (required by the gate; the precedent every later task follows) |
| W2-T3-03 | backends/ezdxf_backend.py | `base_x`/`base_y` only `float()`-cast; NaN written into the BLOCK record. | fixed-in-task-4 |
| W2-T3-04 | backends/ezdxf_backend.py | A primitive layer is written without a layer-table entry; COM would fail mid-loop. | fixed-in-task-4 |
| W2-T3-05 | README.md | Same as W2-T3-01. | fixed-in-track-A |
| W2-T3-06 | backends/ezdxf_backend.py | ATTDEF linetype stays BYLAYER on ezdxf vs ByBlock on COM; neither sets lineweight ByBlock. | deferred-1.7 (cosmetic; layer-0 members resolve to the INSERT's layer) |
| W2-T3-07 | backends/ezdxf_backend.py | `attdefs or []` turns `{}`/`''` into "no ATTDEFs"; a non-str `name` raises AttributeError. | deferred-1.7 |
| W2-T3-08 | backends/ezdxf_backend.py | `block_insert` without `attributes` carries none of the ATTDEF defaults (COM does). | deferred-1.7 (with W1-T1-08) |
#### Track A Task 4 — XDATA
| ID | File | Finding | Disposition |
|---|---|---|---|
| W2-T4-01 | discovery/aliases.py | `entity_get_xdata` ships `XDLIST` only (XDATA would break the single-route gate). | accepted-limitation (still discoverable; reporting gap) |
| W2-T4-02 | tests/test_tool_registry.py | Snapshot edited outside the file list. | accepted-limitation (gate precedent) |
| W2-T4-03 | backends/ezdxf_backend.py | APPID names are case-sensitive headlessly; `APP_A` and `app_a` become two groups and double-count the budget. | deferred-1.7 |
| W2-T4-04 | backends/com_backend.py | A no-op removal registers the APPID on COM but not on ezdxf. | deferred-1.7 |
| W2-T4-05 | backends/xdata_specs.py | 1004 binary chunks decode to the repr of a bytes object. | deferred-1.7 (hex string) |
| W2-T4-06 | backends/xdata_specs.py | The reserved-app rule blocks reading `ACAD` by name while the unfiltered read returns it; other AutoCAD-owned apps are writable. | deferred-1.7 |
| W2-T4-07 | README.md | Snapshot paragraph contradicted itself; collected-tests drift. | fixed-in-track-A (Task 21) |
#### Track A Task 5 — `engineering/pid/xdata.py`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W2-T5-01 | engineering/pid/__init__.py | Listed as "create" but already existed. | accepted-limitation (nothing missing) |
| W2-T5-02 | tests/test_pid_xdata.py | Sync tests under a module-level `pytest.mark.asyncio` emit PytestWarnings. | deferred-1.7 (hygiene sweep across the pid test modules) |
| W2-T5-03 | spec | Spec payload (`actuator`, 3-element ports, no `seq`) vs plan (`variant`, 5-element ports, `seq`). | accepted-limitation (plan governs; spec text to reconcile) |
| W2-T5-04 | engineering/pid/xdata.py | `encode_payload` uses `allow_nan=True`; a NaN would be serialised as a non-RFC literal. | deferred-1.7 (`allow_nan=False`; no current caller can deliver one after Task 5) |
| W2-T5-05 | engineering/pid/xdata.py | `symbol_payload(params={})` is replaced by the spec's params (truthiness). | deferred-1.7 |
| W2-T5-06 | engineering/pid/xdata.py | Duplicate port names collapse silently (last wins). | deferred-1.7 (refuse in `make_spec`) |
| W2-T5-07 | engineering/pid/xdata.py | A non-dict payload is written and can never be read back. | deferred-1.7 |
| W2-T5-08 | tests/test_pid_xdata.py | No multi-chunk payload round trip through a backend. | deferred-1.7 (test gap; the path was verified by hand) |
#### Track A Task 7 — symbol framework
| ID | File | Finding | Disposition |
|---|---|---|---|
| W2-T7-01 | engineering/pid/symbols.py | Per-type outlines (no circle for computer/plc) deviate from the plan snippet, unreported. | accepted-limitation (the spec's ISA table governs; tests added) |
| W2-T7-02 | tests/test_pid_symbols.py | Unused import dropped, `math` added. | accepted-limitation (no functional effect) |
| W2-T7-03 | engineering/pid/symbols.py | Rear-location dashed lines stop up to 1 mm short on the right. | accepted-limitation (cosmetic; no port, bbox or gate depends on it) |
| W2-T7-04 | engineering/pid/symbols.py | A single radial `signal` port cannot describe the hexagon/square outlines (0.67 mm gap on vertical lines into `computer`). | accepted-limitation (spec §4.4 model; a per-axis reach is a 1.7 item) |
| W2-T7-05 | engineering/pid/symbols.py | `all_specs()` swallows builder ValueErrors and nothing pins the catalogue size. | deferred-1.7 (assert `len(all_specs())`) |
| W2-T7-06 | engineering/pid/symbols.py | `register()` overwrites silently; `list_symbols()` returns variants by reference. | deferred-1.7 |
| W2-T7-07 | engineering/pid/geometry.py | `bbox_of` samples arcs at 1° and can under-report by 3.8e-5·r. | accepted-limitation (sub-lineweight) |
#### Track A Task 8 — valves
| ID | File | Finding | Disposition |
|---|---|---|---|
| W2-T8-01 | plan | The plan file was edited to match the shipped geometry, unreported. | accepted-limitation (the original geometry floated the dome; evidence in the commit) |
| W2-T8-02 | engineering/pid/symbols_valves.py | `check`/`relief` advertise actuator variants they refuse. | deferred-1.7 (variants `["none"]` for those bodies) |
| W2-T8-03 | engineering/pid/symbols_valves.py | The diaphragm *body* dome floats 1 mm above the bow-tie. | deferred-1.7 (cosmetic) |
| W2-T8-04 | engineering/pid/symbols_valves.py | The `signal` port and the TAG/DESC text share x = 0 above the actuator; a signal line crosses the tag. | accepted-limitation (spec §4.7 placement; a tag/line overlap critique is a 1.7 item, with W2-T9-05) |
#### Track A Task 9 — equipment
| ID | File | Finding | Disposition |
|---|---|---|---|
| W2-T9-01 | engineering/pid/symbols_equipment.py | `pd_pump` ports are `in`/`out`, spec says `suction`/`discharge`. | accepted-limitation (plan governs; spec text to reconcile; `pid_from_spec` names ports explicitly) |
| W2-T9-02 | engineering/pid/symbols_equipment.py | `spectacle_blind` cites ISO 10628-2 instead of PIP PIC001; no spec names its figure/table. | deferred-1.7 (provenance strings) |
| W2-T9-03 | engineering/pid/symbols_equipment.py | Same as W2-T9-01. | accepted-limitation |
| W2-T9-04 | engineering/pid/symbols_equipment.py | `build_equipment` ignores `shape` for non-reducers and its signature is narrower than `**options`. | accepted-limitation (`resolve()` validates options; builders are internal) |
| W2-T9-05 | engineering/pid/symbols_equipment.py | TAG/DESC sit on the outward ray of every +Y port. | accepted-limitation (with W2-T8-04) |
| W2-T9-06 | engineering/pid/symbols_equipment.py | `air_cooler` fan cross pokes 0.046 mm outside its circle. | deferred-1.7 (cosmetic, ±1.75) |
| W2-T9-07 | engineering/pid/symbols_equipment.py | Seven origins are not the bbox centre (spec §4.2). | accepted-limitation (deliberate mounting points; spec text to reconcile) |
#### Track A Task 10 — vessels
| ID | File | Finding | Disposition |
|---|---|---|---|
| W2-T10-01 | engineering/pid/geometry.py | `scan_hits` added outside the file list, unreported. | accepted-limitation (additive, tested) |
| W2-T10-02 | engineering/pid/symbols_vessels.py | Stricter size rules and outline-based stubs, unreported. | accepted-limitation (evidence in the commit; plan tests unchanged) |
| W2-T10-03 | spec | `PID_VESSEL_VERTICAL_` vs `PID_VESSEL_VERTICAL_VESSEL_`. | accepted-limitation (plan's general rule governs) |
| W2-T10-04 | engineering/pid/symbols_vessels.py | Non-finite sizes/fractions pass `validate_vessel_params` (NaN spec, `RuntimeError` for inf). | deferred-1.7 (`math.isfinite` next to `> 0`) |
| W2-T10-05 | engineering/pid/symbols_vessels.py | Column tray lines poke through dished heads (up to 1.8 mm). | deferred-1.7 (clip to the outline with `scan_hits`) |
| W2-T10-06 | engineering/pid/symbols_vessels.py | Nozzle fractions near 0/1 on curved/sloped sides give degenerate long stubs from a corner. | deferred-1.7 |
| W2-T10-07 | engineering/pid/symbols_vessels.py | `trays` is unbounded (1 000 000 trays build in 6.3 s / 272 MB). | deferred-1.7 (cap by spacing ≥ 1 mm) |
| W2-T10-08 | engineering/pid/symbols_vessels.py | Two nozzles at the same side+fraction duplicate the stub; `-0.0` hashes to a different block name. | deferred-1.7 |
## Waves 3–4
#### Track A Task 11 — `pid_symbol_list` / `pid_symbol_insert`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W3-T11-01 | engineering/layers.py | `PROCESS-LINE-TEXT` inserted at position 9; spec said "16th entry". | accepted-limitation (count matches; ordinal wording only) |
#### Track A Task 13 — `pid_line_draw`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W3-T13-01 | tests/test_tool_registry.py | Snapshot edited outside the file list. | accepted-limitation (gate precedent) |
| W3-T13-02 | discovery/aliases.py | `PLINE` added to `SHARED_ACAD_COMMANDS`. | accepted-limitation (required by the single-route gate; documented in the allowlist comment) |
| W3-T13-03 | backends/com_backend.py | `e924b5d` touched three files outside the list, unreported (COM `AcDbPolyline` naming, mirrored ports). | accepted-limitation (both were defects; tests added) |
| W3-T13-04 | server.py | `>=` for `≥` in a docstring. | accepted-limitation (cosmetic) |
| W3-T13-05 | engineering/pid/drawlines.py | Line sequence is per current space, not per document (spec §6.3). | deferred-1.7 (document-wide scan of existing lines) |
| W3-T13-06 | engineering/pid/drawlines.py | Every signal line got a fabricated number and label and consumed the process sequence. | fixed-in-task-6 |
| W3-T13-07 | engineering/pid/drawlines.py | A bubble's exit aimed at the far end even with waypoints, so the first segment cut through the bubble. | fixed-in-task-6 |
| W3-T13-08 | engineering/pid/drawlines.py | A foreign/hand-edited payload with a non-dict `from`/`to` raised `TypeError` out of `draw_line`. | fixed-in-task-7 |
| W3-T13-09 | engineering/pid/drawlines.py | (a) crossings tested against chords, not arcs; (b) COM heavy 2DPOLYLINE `Coordinates` read with stride 2. | (a) fixed-in-task-7; (b) deferred-1.7 (heavy polylines the tool never creates) |
| W3-T13-10 | README.md | Snapshot line edited without the collected-tests figure. | fixed-in-track-A (Task 21) |
| W3-T13-11 | server.py | An unknown endpoint handle and a flat waypoint list surface as "Internal error" rather than a named refusal. | deferred-1.7 (with W1-T12-04) |
#### Track A Task 14 — `pid_graph`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W3-T14-01 | engineering/pid/graph.py | Block-name keywords match whole tokens, not substrings (spec §9.2); `GATEVALVE` no longer classifies. | accepted-limitation (the plan's own test demands it; documented in code) |
| W3-T14-02 | backends/ezdxf_backend.py | A public `bulges` key was added to every LWPOLYLINE `entity_get`, unreported. | accepted-limitation (additive; spec §9.4 "geometry from the drawing"; now consumed by Task 7) |
| W3-T14-03 | tests/test_tool_registry.py | Snapshot edited outside the file list. | accepted-limitation (gate precedent) |
| W3-T14-04 | engineering/pid/graph.py | Edge dict carries `length_approximate` and (with geometry) `bulges` beyond the spec. | accepted-limitation (additive) |
| W3-T14-05 | tests/test_pid_graph.py | Three sync tests under the asyncio mark warn. | deferred-1.7 (hygiene, with W2-T5-02) |
| W3-T14-06 | engineering/pid/graph.py | `_Edge.segments` omits a closed polyline's closing edge while `length` includes it. | deferred-1.7 |
| W3-T14-07 | engineering/pid/graph.py | The dangling `nearest` hint never names a radial (bubble) port. | deferred-1.7 |
| W3-T14-08 | engineering/pid/graph.py | Text-tag fallback searched a circle, not "above the box". | fixed-in-track-A (`d17a261`, `6ad3734`: tags above the box, `LINE_NUMBER_RE` excluded) |
| W3-T14-09 | engineering/pid/graph.py | `scope="all"` restores `current` but not the paper-space layout binding. | deferred-1.7 |
| W3-T14-10 | engineering/pid/graph.py | A −Z-extruded catalogue INSERT reported mirror-image ports at confidence 1.0. | fixed-in-track-A (`6ad3734`: both engines report `mirrored`; `transform_port(mirrored=)`) |
| W3-T14-11 | engineering/pid/graph.py | A `PID_*` INSERT whose XDATA was stripped falls to the 0.6 heuristic with no ports (spec says catalogue 1.0). | deferred-1.7 |
| W3-T14-12 | README.md | Release-consistency gate red on the collected-tests figure. | fixed-in-track-A (Task 21) |
| W3-T14-13 | engineering/pid/symbols.py | `6ad3734` touched five files outside the list; `transform_port` grew `mirrored`/`strict`. | accepted-limitation (justified; interface kept by keyword defaults) |
| W3-T14-14 | engineering/pid/graph.py | `_text_near` accepts a baseline half a height inside the box and excludes line-number texts — beyond the spec wording. | accepted-limitation (documented, tested) |
| W3-T14-15 | engineering/pid/graph.py | Node dicts gained `y_scale`, `mirrored`, `notes`, `space`, `description`; junctions `space`. | accepted-limitation (additive) |
| W3-T14-16 | docs (plan/spec) | `ruff format --check` was red on the plan and spec Markdown. | fixed-in-track-A (Task 21 formatted them) |
| W3-T14-17 | spec | §9.1 model not extended for the added node/junction keys. | deferred-1.7 (docs) |
| W3-T14-18 | engineering/pid/symbols.py | `strict=False` keeps a diagonal port's local angle under non-uniform scale. | accepted-limitation (unreachable: all 243 catalogue ports are axis-aligned) |
| W3-T14-19 | backends/ezdxf_backend.py | `mirrored` is `True` for a tilted INSERT with a negative-z normal (COM twin too). | deferred-1.7 (`flat and z < 0`) |
#### Track A Task 15 — deliverables
| ID | File | Finding | Disposition |
|---|---|---|---|
| W3-T15-01 | tests/test_pid_deliverables.py | Four plan deviations (line-list rows, `_far_ends`, CSV refusal, `deliverable()` ValueError) unreported. | accepted-limitation (each stronger than the plan; the plan's own numbers were wrong) |
| W3-T15-02 | tests/test_tool_registry.py | Snapshot edited outside the file list. | accepted-limitation (gate precedent) |
| W3-T15-03 | tests/_probe_task15.py | A stray probe file appeared during review. | fixed-in-track-A (not in the tree) |
| W3-T15-04 | engineering/pid/deliverables.py | `natural_key` crashes on `str.isdigit()` runs that are not `\d` (①, ²). | deferred-1.7 (`isdecimal`) |
| W3-T15-05 | engineering/pid/deliverables.py | `_far_ends` resolves junctions at the ends of the instrument's line only, not on it. | deferred-1.7 |
| W3-T15-06 | engineering/pid/deliverables.py | `_tag_of` writes the internal junction id into a tag column; untagged node ends are `None` vs handle in the index. | deferred-1.7 |
| W3-T15-07 | engineering/pid/deliverables.py | `write_csv` edge cases: 0-byte file for zero rows, `csv_path=''` treated as None, directory path escapes as PermissionError, dead `mkdir`, late validation. | deferred-1.7 |
| W3-T15-08 | engineering/pid/deliverables.py | CSV formula injection: cells starting with `=+-@` are written verbatim. | deferred-1.7 (first item of the 1.7 deliverables pass) |
| W3-T15-09 | server.py | The three deliverable tools expose only `tolerance`/`scope`, not `label_search`/`include_foreign`. | deferred-1.7 |
| W3-T15-10 | engineering/pid/deliverables.py | `signal_lines` counts every edge on a bubble regardless of class. | deferred-1.7 |
#### Track A Task 16 — critique focuses
| ID | File | Finding | Disposition |
|---|---|---|---|
| W3-T16-01 | tests/test_pid_critique.py | The refine test calls `refine_drawing` (no backend `drawing_refine` exists), unreported. | accepted-limitation (the plan's call could not run; stricter test) |
| W3-T16-02 | engineering/pid/critique.py | `has_pid_content` guard instead of `not graph['nodes']`, unreported. | accepted-limitation (spec intent; pinned by tests) |
| W3-T16-03 | engineering/pid/critique.py | `_incompatible` skips inferred ports; `_dangling` reports unknown_block ends — beyond the plan. | accepted-limitation (spec §9.4 / §11.1; tested) |
| W3-T16-04 | tests/test_pid_critique.py | Two sync tests warn under the asyncio mark. | deferred-1.7 (hygiene) |
| W3-T16-05 | engineering/pid/critique.py | `_untagged` tests the composed tag, not FUNC/LOOP; an empty LOOP files as `pid_illegal_tag`. | deferred-1.7 |
| W3-T16-06 | engineering/pid/critique.py | A mechanical block whose name holds a keyword token (`OIL_FILTER`) counts as P&ID content and can fail finalize. | accepted-limitation (spec §9.2 mandates the heuristic; README "Known limitations" states it) |
| W3-T16-07 | engineering/pid/critique.py | `_duplicate_tags` scores a text-derived (guessed) tag as a gate-failing error. | deferred-1.7 (warning for `tag_source: text`) |
#### Track A Task 17 — `pid_from_spec`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W4-T17-01 | tests/test_pid_from_spec.py | Four reported deviations verified legitimate (save_path, score dict, async test, snapshot). | accepted-limitation |
| W4-T17-02 | engineering/pid/spec.py | The module is a refactor of the plan's listing; interfaces preserved. | accepted-limitation |
| W4-T17-03 | README.md | `pid-showcase.png` rendered but not referenced. | fixed-in-track-A (README line 20 embeds it) |
| W4-T17-04 | tests/test_release_consistency.py | Collected-tests drift. | fixed-in-track-A (Task 21) |
| W4-T17-05 | engineering/pid/spec.py | `_num` accepts NaN/inf; an INSERT lands at `[nan, 0]` and `build_graph` crashes. | fixed-in-task-5 |
| W4-T17-06 | engineering/pid/spec.py | `run_spec` catches `Exception` only; a cancellation leaves an open transaction. | fixed-in-task-5 |
| W4-T17-07 | engineering/pid/spec.py | `dry_run` skips `place_symbol`'s option checks (`scale=-1` approved; wrong item blamed). | deferred-1.7 (`validate_spec` owns scale/fail/actuator rules) |
| W4-T17-08 | engineering/pid/spec.py | `dry_run` crossings count the spec's own lines only, unlabelled. | deferred-1.7 (`crossings_scope` field) |
| W4-T17-09 | engineering/pid/spec.py | Nested transactions: ezdxf nests, COM refuses; `drawing_plan` is set before the refusal. | deferred-1.7 |
| W4-T17-10 | engineering/pid/spec.py | The `sheet` block is not validated (unknown layer set / sheet size / keys accepted). | deferred-1.7 |
| W4-T17-11 | engineering/pid/spec.py | String-typed optional keys unvalidated (`arrow: "no"` truthy; `number: 123` bare TypeError; `id` with a dot). | deferred-1.7 |
| W4-T17-12 | README.md | README showcase half incomplete; gate red. | fixed-in-track-A (Task 21) |
#### Track A Task 19 — tag parse, resources, prompt
| ID | File | Finding | Disposition |
|---|---|---|---|
| W4-T19-01 | tests/test_tool_registry.py | Snapshot edited outside the file list. | accepted-limitation (gate precedent) |
| W4-T19-02 | scripts/render_pid_catalog.py | Renders through its own matplotlib path instead of `view_screenshot`, unreported. | accepted-limitation (byte-reproducible output; interface kept) |
| W4-T19-03 | README.md | Lines 23/106 still said 154 / 6 resources. | fixed-in-track-A (Task 21) |
| W4-T19-04 | server.py | The prompt's OFF-PAGE example emits a tag warning every time it is followed (`TO P&ID-002` parsed as ISA-5.1). | deferred-1.7 (skip `parse_tag` for connectors at insert) |
| W4-T19-05 | engineering/pid/tags.py | `equipment_prefixes={}` discarded; non-string values pass. | deferred-1.7 (with W1-T6-05) |
| W4-T19-06 | README.md | Release gate red on the collected-tests figure. | fixed-in-track-A (Task 21) |
#### Track A Task 18 — `TOOL_PACKS`
| ID | File | Finding | Disposition |
|---|---|---|---|
| W4-T18-01 | tests/test_tool_packs.py | A sync test warns under the asyncio mark. | deferred-1.7 (hygiene) |
| W4-T18-02 | README.md / config.py / .env.example | Token table and prose said 47 while lean has 50. | README: fixed-in-track-A (Task 21); `config.py:99` / `.env.example:52` "~47": deferred-1.7 (docs) |
| W4-T18-03 | server.py | `TOOL_PACKS=all,mech3000` drops the unknown entry without a warning or `ignored`. | deferred-1.7 |
| W4-T18-04 | server.py | `_enabled_packs()` runs twice per apply; unknown entries warn twice and duplicate in `ignored`. | deferred-1.7 |
| W4-T18-05 | tests/test_tool_packs.py | The pack test compares against a hand-copied set, never the registry. | deferred-1.7 (assert against `_registered_tools()` tags) |
| W4-T18-06 | server.py | Under `TOOL_PACKS=core` the P&ID prompt and resources are still advertised. | accepted-limitation (spec §12 scopes packs to tools) |
#### Track A Task 20 — benchmark v4
| ID | File | Finding | Disposition |
|---|---|---|---|
| W4-T20-01 | scripts/check_doc_numbers.py | The gate short-circuited on the 29-vs-26 A/B drift. | fixed-in-track-A (Task 21 regenerated the A/B report) |
| W4-T20-02 | CLAUDE.md | An extra clause beyond the dictated sentence. | accepted-limitation (accurate) |
| W4-T20-03 | benchmarks/README.md | "16/16 on the v4 matrix" has no published artifact behind it. | deferred-1.7 (publish a v4 reference report at the 1.6 release) |
| W4-T20-04 | tests/test_benchmark_v4.py | No test executes `_task_pid_roundtrip`. | deferred-1.7 (Task 6 of the hardening plan runs it by hand) |
| W4-T20-05 | benchmarks/README.md | A/B rows said 29 checks over a 26-check report. | fixed-in-track-A (Task 21) |
#### Track A Task 21 — docs and smoke
| ID | File | Finding | Disposition |
|---|---|---|---|
| W4-T21-01 | scripts/smoke_pid_com.py | Not re-run live during review (no AutoCAD running). | accepted-limitation (the implementer's live run is recorded in the CHANGELOG) |
| W4-T21-02 | README.md | "Known limitations of the P&ID track" was not a plan item. | accepted-limitation (claims verified against the code) |
| W4-T21-03 | docs (plan) | The plan file was ruff-formatted outside the file list. | accepted-limitation (mechanical) |
| W4-T21-04 | benchmarks/README.md | Tables moved for the new gate labels. | accepted-limitation (required by `check_doc_numbers.py`) |
| W4-T21-05 | CHANGELOG.md | "12.5 % short on a semicircular jump" is the v1.5 square figure; the tested geometry is 10.25 %. | deferred to the final docs task of Track E (CHANGELOG `Unreleased` edit) |
| W4-T21-06 | README.md | The snapshot is labelled v1.5 while quoting the 1.6-dev surface. | accepted-limitation (pinned to pyproject's minor until the 1.6 bump) |
| W4-T21-07 | scripts/smoke_pid_com.py | Exit code checks `dangling` only; critique/confidence/xdata are printed, not asserted. | deferred-1.7 |
| W4-T21-08 | CHANGELOG.md | `LAYER_SET_INTENT_MISMATCH` is claimed for `drawing_plan`, which only appends a warning string. | deferred to the final docs task of Track E |
## Totals
165 rows. Fixed in this track (Tasks 1–7): 15 · fixed inside Track A: 17 · accepted with a reason: 62 · deferred (1.7 or the Track E docs task): 71.
+88 -26
View File
@@ -11,6 +11,7 @@ from .lines import (
DEFAULT_NUMBER_FORMAT,
LINE_CLASSES,
PID_LINE_LAYERS,
aim_points,
count_crossings,
format_line_number,
label_placement,
@@ -105,13 +106,57 @@ def _vertices(info) -> list[tuple[float, float]]:
return [(float(p[0]), float(p[1])) for p in info.properties.get("points") or []]
def _bulges(info, count: int) -> list[float]:
"""Per-vertex bulges as the engine reports them (both engines put a
``bulges`` list next to ``points``); straight when it reports none."""
raw = info.properties.get("bulges") or []
out = []
for value in raw[:count]:
try:
out.append(float(value or 0.0))
except (TypeError, ValueError):
out.append(0.0)
return out + [0.0] * (count - len(out))
def payload_port_refs(payload) -> list[tuple[str, str]]:
"""``(handle, port)`` for each well-formed ``from``/``to`` in a line payload.
A foreign or hand-edited payload can carry anything under those keys
(``"garbage"``, ``None``, a number); reading it used to raise
``TypeError`` out of ``draw_line``. Reader results never raise: a
malformed end is simply not a reference.
"""
refs = []
if not isinstance(payload, dict):
return refs
for key in ("from", "to"):
ref = payload.get(key)
if not isinstance(ref, dict):
continue
handle, port = ref.get("handle"), ref.get("port")
if isinstance(handle, str) and isinstance(port, str):
refs.append((handle, port))
return refs
def payload_seq(payload) -> int | None:
"""The payload's sequence number when it is a genuine int (``True`` is not)."""
if not isinstance(payload, dict):
return None
seq = payload.get("seq")
return seq if type(seq) is int else None
async def existing_pid_lines(backend: AutoCADBackend) -> list[dict]:
"""Every LINE/polyline on a P&ID line layer, with its payload when it has one.
Listed without a type filter: the two engines name a lightweight polyline
differently (``_POLY_TYPES``), and a filter spelled for one of them is blind
on the other — every auto-built number would restart at 1 and no crossing
or port reuse would ever be reported there.
or port reuse would ever be reported there. ``vertices`` are the raw
vertices; ``bulges`` (one per vertex, DXF convention) let the crossing test
follow an arc instead of its chord.
"""
out = []
offset = 0
@@ -123,17 +168,26 @@ async def existing_pid_lines(backend: AutoCADBackend) -> list[dict]:
if info.layer.upper() not in PID_LINE_LAYERS:
continue
payload = await read_payload(backend, info.handle) if info.type in _POLY_TYPES else None
out.append({"handle": info.handle, "vertices": _vertices(info), "payload": payload})
vertices = _vertices(info)
out.append(
{
"handle": info.handle,
"vertices": vertices,
"bulges": _bulges(info, len(vertices)),
"payload": payload,
}
)
if len(page) < _PAGE:
break
offset += _PAGE
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."""
def _anchor(endpoint: dict, aim: tuple[float, float]) -> tuple[tuple[float, float], float | None]:
"""Where the line actually starts/ends: on the circle for radial ports,
leaving along the axis towards ``aim`` (see ``aim_points``)."""
if endpoint["radius"] > 0:
axis = snap_axis(other["x"] - endpoint["x"], other["y"] - endpoint["y"])
axis = snap_axis(aim[0] - endpoint["x"], aim[1] - endpoint["y"])
ux, uy = AXIS[axis]
return (
endpoint["x"] + ux * endpoint["radius"],
@@ -176,38 +230,46 @@ async def draw_line(
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)
s_aim, e_aim = aim_points((start["x"], start["y"]), (end["x"], end["y"]), route_mode)
s_point, s_dir = _anchor(start, s_aim)
e_point, e_dir = _anchor(end, e_aim)
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])
crossings = count_crossings(
path,
[ln["vertices"] for ln in existing],
bulges=[ln["bulges"] 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"])
used.update(payload_port_refs(ln["payload"]))
seq = payload_seq(ln["payload"])
if seq is not None:
max_seq = max(max_seq, 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
if cls.kind == "signal":
# An ISA-5.1 signal line carries no pipe line number: no fabricated
# number, no label, and the process sequence is left alone. A
# verbatim ``line_number`` is still honoured (and labelled).
seq = None
else:
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(
+19 -2
View File
@@ -12,7 +12,7 @@ import re
from dataclasses import dataclass, field
from typing import TYPE_CHECKING
from .lines import LAYER_TO_CLASS, PID_LINE_LAYERS
from .lines import LAYER_TO_CLASS, LINE_CLASSES, PID_LINE_LAYERS
from .symbols import Port, transform_port
from .tags import parse_tag
from .xdata import read_payload
@@ -55,6 +55,15 @@ CONFIDENCE = {"catalog": 1.0, "xdata": 0.95, "heuristic": 0.6, "inferred": 0.3}
_PAGE = 1000
def _is_signal_class(line_class: str | None) -> bool:
"""True for a class that is a signal in the ISA-5.1 sense (`signal_unknown`
is the layer-classified one that LINE_CLASSES does not list)."""
if line_class == "signal_unknown":
return True
cls = LINE_CLASSES.get(line_class) if line_class else None
return bool(cls and cls.kind == "signal")
@dataclass
class _Node:
id: str
@@ -684,7 +693,15 @@ async def build_graph(
if found:
node.tag, node.tag_source = found, "text"
for edge in edges.values():
if edge.line_number:
if edge.line_number or edge.payload or _is_signal_class(edge.line_class):
# The label search is a fallback for a line that carries no
# record of its own. An engine-authored line (payload) is that
# record — its `number: null` is the answer, not an invitation to
# adopt the nearest pipe's label. A signal line never has a pipe
# line number (ISA-5.1), and LINE_NUMBER_RE is the pipe grammar,
# so any match near it belongs to a neighbouring pipe: the
# ordinary transmitter-to-controller run 12 mm above a header
# would otherwise be listed under the header's number.
continue
a, b = max(zip(edge.vertices, edge.vertices[1:], strict=False), key=lambda s: math.dist(*s))
found = _text_near(
+140 -7
View File
@@ -198,6 +198,21 @@ def route(start, start_dir, end, end_dir, stub: float = 5.0, mode="auto") -> lis
return valid[0]
def aim_points(start, end, mode) -> tuple[Point, Point]:
"""What each end of a line aims at: the other end, or — with waypoints —
the first waypoint for the start and the last one for the end.
A radial (bubble) port leaves its circle along the axis towards its aim
point. Aiming at the far end while the route is a waypoint list put the
exit on the wrong side of the bubble, so the first segment cut through it.
"""
if isinstance(mode, (list, tuple)) and len(mode) > 0:
return _finite_point("waypoints[0]", mode[0]), _finite_point(
f"waypoints[{len(mode) - 1}]", mode[-1]
)
return _finite_point("end", end), _finite_point("start", start)
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]
@@ -223,6 +238,72 @@ def marker_positions(vertices, min_len: float = 15.0) -> list[tuple[float, float
return out
#: Max chord deviation (mm) when a bulged polyline edge is flattened into
#: chords (``flatten_bulges``) — a tenth of the thinnest ISO 128 lineweight.
#: The crossing count does **not** flatten: it tests the exact arc.
FLATTEN_SAGITTA = 0.05
def _arc_of_bulge(a: Point, b: Point, bulge: float) -> tuple[float, float, float, float, float]:
"""``(cx, cy, radius, start, theta)`` of the arc a→b with the DXF bulge
``tan(sweep/4)``: ``theta`` is the signed sweep in radians (positive
counter-clockwise) and ``start`` the angle of ``a`` about the centre.
Same arithmetic as ``engineering/measure.py`` (sweep ``4·atan(bulge)``,
radius ``chord / 2·sin(|sweep|/2)``) and ``graph._point_segment_distance``
(centre on the chord's left normal, ``radius - sagitta`` carrying it across
the chord past a semicircle). The caller guarantees a non-zero bulge and a
non-degenerate chord.
"""
chord = _dist(a, b)
theta = 4.0 * math.atan(bulge) # signed sweep, radians
radius = chord / (2.0 * math.sin(abs(theta) / 2.0))
sag = abs(bulge) * chord / 2.0
nx, ny = -(b[1] - a[1]) / chord, (b[0] - a[0]) / chord # left of a→b
offset = (radius - sag) * (1.0 if bulge > 0 else -1.0)
cx = (a[0] + b[0]) / 2.0 + offset * nx
cy = (a[1] + b[1]) / 2.0 + offset * ny
start = math.atan2(a[1] - cy, a[0] - cx)
return cx, cy, radius, start, theta
def flatten_bulges(points, bulges, sagitta: float = FLATTEN_SAGITTA) -> list[Point]:
"""The polyline as straight segments, every bulged edge replaced by chords
that deviate from the arc by at most ``sagitta``.
Arc arithmetic from ``_arc_of_bulge``. Checked against
``ezdxf.math.bulge_to_arc`` + ``ConstructionArc.flattening`` in
``tests/test_pid_lines.py``: identical vertex count and positions to 1e-12
for bulges 0.2 … 2.0 of either sign. A straight edge (bulge 0) is passed
through; ``bulges`` shorter than the edge count is padded with zeros.
Not used by the crossing count: a flattened chain has artificial vertices,
and ``segments_cross`` treats a hit on any vertex as a junction — a
semicircle flattens to an even chord count, so a line through its apex
(the axis of symmetry, exactly where a P&ID line meets a crossing jump)
would be dropped by both chords. ``count_crossings`` tests the exact arc.
"""
pts = [(float(x), float(y)) for x, y in points]
if len(pts) < 2:
return pts
out: list[Point] = []
for i, (a, b) in enumerate(zip(pts, pts[1:], strict=False)):
out.append(a)
bulge = float(bulges[i]) if i < len(bulges) and bulges[i] else 0.0
if abs(bulge) < 1e-12 or _dist(a, b) < 1e-12:
continue
cx, cy, radius, start, theta = _arc_of_bulge(a, b, bulge)
if sagitta >= radius:
segments = 1
else:
segments = max(1, math.ceil(abs(theta) / (2.0 * math.acos(1.0 - sagitta / radius))))
for k in range(1, segments):
angle = start + theta * k / segments
out.append((cx + radius * math.cos(angle), cy + radius * math.sin(angle)))
out.append(pts[-1])
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])
@@ -234,15 +315,67 @@ def segments_cross(p1, p2, q1, q2) -> bool:
return d1 * d2 < -_EPS and d3 * d4 < -_EPS
def count_crossings(vertices, others) -> int:
def segment_arc_crossings(p1, p2, a, b, bulge: float) -> int:
"""Proper crossings (0, 1 or 2) of the straight segment p1→p2 with the
exact arc a→b of DXF ``bulge``; a zero bulge is the chord test.
The same junction rule as ``segments_cross``: a hit within ``_EPS`` (mm)
of either end of the segment or of the arc is a T-touch or a shared
vertex, not a crossing, and a tangent (the line reaches no deeper than
``_EPS`` into the circle) touches without crossing. Only the arc's own
two ends are junction points — nothing along the sweep is a vertex.
"""
if abs(bulge) < 1e-12 or _dist(a, b) < 1e-12:
return 1 if segments_cross(p1, p2, a, b) else 0
cx, cy, radius, start, theta = _arc_of_bulge(a, b, bulge)
dx, dy = p2[0] - p1[0], p2[1] - p1[1]
length_sq = dx * dx + dy * dy
if length_sq < 1e-24:
return 0
length = math.sqrt(length_sq)
fx, fy = p1[0] - cx, p1[1] - cy
# |p1 + t·d − c|² = r² → A t² + B t + C = 0 with A = |d|²
coeff_b = 2.0 * (fx * dx + fy * dy)
coeff_c = fx * fx + fy * fy - radius * radius
disc = coeff_b * coeff_b - 4.0 * length_sq * coeff_c
if disc <= 0.0:
return 0
root = math.sqrt(disc)
if root / (2.0 * length) <= _EPS:
return 0 # tangent: the half-chord inside the circle is nothing
sweep = abs(theta)
sign = 1.0 if theta > 0 else -1.0
count = 0
for numerator in (-coeff_b - root, -coeff_b + root):
t = numerator / (2.0 * length_sq)
if t * length <= _EPS or (1.0 - t) * length <= _EPS:
continue # the segment ends on the arc: a T-touch
x, y = p1[0] + t * dx, p1[1] + t * dy
along = (sign * (math.atan2(y - cy, x - cx) - start)) % (2.0 * math.pi)
if along * radius <= _EPS or abs(sweep - along) * radius <= _EPS:
continue # the arc's own end: the polyline's vertex, a junction
if along > sweep:
continue # on the circle but outside the arc
count += 1
return count
def count_crossings(vertices, others, bulges=None) -> int:
"""Proper crossings of the straight chain ``vertices`` with every chain in
``others``. ``bulges[i]`` is the per-vertex DXF bulge list of ``others[i]``
(the arc leaving each vertex; short lists are zero-padded, ``None`` means
every chain is straight): a bulged edge is tested as its exact arc, never
as its chord or as flattened chords.
"""
pts = [(float(x), float(y)) for x, y in vertices]
count = 0
for a, b in zip(pts, pts[1:], strict=False):
for other in others:
opts = [(float(x), float(y)) for x, y in other]
for c, d in zip(opts, opts[1:], strict=False):
if segments_cross(a, b, c, d):
count += 1
for index, other in enumerate(others):
opts = [(float(x), float(y)) for x, y in other]
obulges = list(bulges[index]) if bulges is not None and index < len(bulges) else []
for a, b in zip(pts, pts[1:], strict=False):
for j, (c, d) in enumerate(zip(opts, opts[1:], strict=False)):
bulge = float(obulges[j]) if j < len(obulges) and obulges[j] else 0.0
count += segment_arc_crossings(a, b, c, d, bulge)
return count
+71 -11
View File
@@ -3,18 +3,24 @@
from __future__ import annotations
import copy
import logging
import math
from typing import TYPE_CHECKING
import anyio
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 AXIS, LINE_CLASSES, count_crossings, route, snap_axis
from .lines import AXIS, LINE_CLASSES, aim_points, count_crossings, route, snap_axis
from .symbols import resolve, transform_port
if TYPE_CHECKING:
from backends.base import AutoCADBackend
log = logging.getLogger(__name__)
EXAMPLE_SPEC: dict = {
"sheet": {
"layer_set": "pid",
@@ -135,9 +141,24 @@ _LINE_KEYS = {
def _num(value, where: str) -> float:
"""A finite number, or a ``ValueError`` naming the path.
``float("nan")`` used to pass and be placed as an INSERT at ``[nan, 0]``;
the transaction committed and ``build_graph`` then crashed on the drawing
with no path in its message. Only in-process callers can deliver a
non-finite (the MCP pipeline coerces them first), but ``run_spec`` is a
public interface and its docstring promises nothing malformed reaches
the drawing.
"""
if isinstance(value, bool) or not isinstance(value, (int, float)):
raise ValueError(f"{where} must be a number")
return float(value)
try:
number = float(value)
except OverflowError as exc: # a 400-digit int is a number but not a float
raise ValueError(f"{where} must be a finite number") from exc
if not math.isfinite(number):
raise ValueError(f"{where} must be a finite number")
return number
def _ref(value, where: str, ids: set[str]) -> tuple[str, str | None]:
@@ -290,9 +311,9 @@ def _plan_lines(norm: dict) -> list[dict]:
raise ValueError(f"{ref}: no port {port!r}; ports: {', '.join(entry['ports'])}")
return entry["ports"][port]
def anchor(ep: dict, other: dict):
def anchor(ep: dict, aim):
if ep["radius"] > 0:
axis = snap_axis(other["x"] - ep["x"], other["y"] - ep["y"])
axis = snap_axis(aim[0] - ep["x"], aim[1] - 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"]
@@ -301,11 +322,11 @@ def _plan_lines(norm: dict) -> list[dict]:
for index, (where, line) in enumerate(_all_lines(norm)):
try:
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")
)
mode = line.get("route", "auto")
s_aim, e_aim = aim_points((start["x"], start["y"]), (end["x"], end["y"]), mode)
s, sd = anchor(start, s_aim)
e, ed = anchor(end, e_aim)
vertices = route(s, sd, e, ed, stub=float(line.get("stub", 5.0)), mode=mode)
except ValueError as exc:
raise ValueError(f"{where}: {exc}") from exc
crossings = count_crossings(vertices, [p["vertices"] for p in planned])
@@ -422,8 +443,47 @@ async def run_spec(backend: AutoCADBackend, spec: dict, dry_run: bool = False) -
except ValueError as exc:
raise ValueError(f"{where}: {exc}") from exc
crossings_total += drawn["crossings"]
except Exception:
await backend.transaction_rollback()
except BaseException as exc:
# ``Exception`` alone let a client cancellation (``CancelledError`` is
# a ``BaseException``) leave the placed symbols in the drawing inside
# an open transaction — on COM with the undo mark still open, so every
# later ``pid_from_spec`` was refused until a manual rollback.
#
# The rollback is shielded because the MCP SDK cancels a request by
# cancelling an anyio scope, and anyio cancellation is level-triggered:
# ``task.cancel()`` is re-issued on every loop iteration while the task
# is still inside the cancelled scope. A bare ``await`` here would be
# cancelled at its first suspension, before the rollback body ran —
# measured through the real transport: five symbols left in an open
# transaction, and on COM the undo mark left open while the backend's
# flag already said "no transaction". A native ``Task.cancel()`` is
# delivered once and clears, so it was never the case that mattered.
#
# Once the rollback body runs after a cancellation it can also *fail*
# (the COM hard timeout, ``_safe_send_command``'s own deadline, an
# ezdxf quarantine refusal). That failure must not replace the
# cancellation: the SDK has already answered the cancelled request,
# and only a cancellation exception is suppressed by its handler — an
# ordinary error makes it respond a second time, which trips
# ``assert not self._completed`` in ``mcp.shared.session`` and kills
# the whole session (measured through the real transport). So after a
# cancellation the rollback error is logged and the cancellation is
# what propagates; after any other error the rollback error propagates
# as before, with the original chained as its context.
cancelled = isinstance(exc, anyio.get_cancelled_exc_class())
with anyio.CancelScope(shield=True):
try:
await backend.transaction_rollback()
except Exception as rollback_exc:
if not cancelled:
raise
log.error(
"pid_from_spec: rollback after a cancelled request failed (%s: %s); "
"the sheet may be half-drawn - check system_status and "
"transaction_rollback before retrying",
type(rollback_exc).__name__,
rollback_exc,
)
raise
await backend.transaction_commit()
graph = await build_graph(backend)
+94 -19
View File
@@ -1656,7 +1656,14 @@ async def entity_create_block_ref(
layer: Annotated[str | None, "Layer name"] = None,
ctx: Context = None,
) -> dict:
"""Insert a block reference (instance of an existing block definition)."""
"""Insert a block reference (instance of an existing block definition).
Refused before any write, on both engines: a name that is not a block
definition in this drawing (`block 'X' is not defined` — check
`block_list`), a layout block (`*Model_Space` / `*Paper_Space`, a
reference cycle) and a non-string name. The same gate as `block_insert`;
this tool is the no-attributes form.
"""
await ctx.debug(f"Inserting block '{name}' at ({x},{y})")
result = await _backend(ctx).entity_create_block_ref(
name, x, y, scale_x, scale_y, rotation, layer
@@ -2410,7 +2417,9 @@ async def text_find_replace(
never searched" are different answers. Block *definitions* are included, so
the next insert does not reintroduce the old text. DIMENSION text is out of
scope: its text field holds the `<>` override placeholder rather than the
measurement, so editing it would break the association.
measurement, so editing it would break the association. A multi-line
attribute is matched and rewritten as the whole value (`Line1\\PLine2`),
the same value `block_get_attributes` reports and `block_explode` bursts.
"""
await ctx.info(f"Replacing {find!r} with {replace!r}{' (dry run)' if dry_run else ''}")
return await _backend(ctx).text_find_replace(find, replace, layer or None, match_case, dry_run)
@@ -3002,7 +3011,15 @@ async def block_insert(
layer: Annotated[str | None, "Layer name"] = None,
ctx: Context = None,
) -> dict:
"""Insert a block and optionally set attribute values."""
"""Insert a block and optionally set attribute values.
Refused before any write, on both engines: an undefined or layout block
name, and an attribute value that is not text — a string is written as
is (one line), an int or finite float as its plain digits (`101` →
`"101"`), while `null`, booleans, lists and objects are refused naming
the tag (they used to be written as their Python repr). Values for tags
the block does not define are ignored.
"""
await ctx.info(f"Inserting block '{name}' at ({x},{y})")
result = await _backend(ctx).block_insert(
name, x, y, scale_x, scale_y, rotation, attributes, layer
@@ -3019,7 +3036,27 @@ async def block_explode(
handle: Annotated[str, "Block reference (INSERT) entity handle"],
ctx: Context = None,
) -> dict:
"""Explode a block reference into its individual component entities."""
"""Explode a block reference into its component entities, keeping its
attribute text (the Express Tools BURST rule, on both engines).
Every attached ATTRIB becomes a TEXT with the same value, placement,
height, rotation and layer (an invisible attribute becomes an invisible
TEXT; a multi-line attribute becomes an MTEXT carrying every line), and
a constant attribute (which has no ATTRIB) becomes a TEXT of its value
too; AutoCAD's plain EXPLODE would keep only the tag-name placeholders
and drop the values. Everything lands in the layout that owns the
reference (model space or its paper-space sheet), whichever tab is
current. Returns `inserted_handles` (the geometry), `attribute_texts`
(one TEXT or MTEXT handle per attribute), `exploded_handle` and
`backend`. Refused, with nothing written: a handle that is not a block
reference, a reference nested inside a block definition (explode the
outer reference instead), an external reference (bind it first), a
MINSERT grid, and -- headless only, capability `explode_opaque_members`
-- a block holding a member ezdxf cannot transform (an OLE2FRAME logo, a
VIEWPORT, a proxy entity without proxy graphics), named by member; the
live engine hands those to AutoCAD. Not undoable except through
`drawing_undo` / a transaction.
"""
await ctx.warning(f"Exploding block reference {handle}")
return await _backend(ctx).block_explode(handle)
@@ -3033,7 +3070,11 @@ async def block_get_attributes(
handle: Annotated[str, "Block reference (INSERT) entity handle"],
ctx: Context = None,
) -> dict:
"""Get all attribute values from a block reference as {TAG: value} dict."""
"""Get all attribute values from a block reference as {TAG: value} dict.
A multi-line attribute reports its whole content with `\\P` between the
lines (AutoCAD's `TextString`), on both engines.
"""
return await _backend(ctx).block_get_attributes(handle)
@@ -3050,7 +3091,17 @@ async def block_set_attributes(
attributes: Annotated[dict, "Attribute values to update: {TAG: new_value}"],
ctx: Context = None,
) -> dict:
"""Update attribute values in a block reference."""
"""Update attribute values in a block reference.
Same value rule as `block_insert`: strings and numbers are written,
`null` / booleans / lists / objects are refused naming the tag before
anything changes. `updated_tags` lists the tags that exist on the
reference and were written; unknown tags are ignored.
A multi-line attribute takes `Line1\\PLine2` and keeps every line; the
value written is the one `block_get_attributes` reads back and
`block_explode` bursts, on both engines.
"""
return await _backend(ctx).block_set_attributes(handle, attributes)
@@ -3109,19 +3160,30 @@ async def block_define(
overwrite: Annotated[
bool, "Replace the contents of an existing definition of this name"
] = False,
create_layers: Annotated[
bool, "Create primitive layers that do not exist yet (default: refuse and name them)"
] = 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})`.
entry refuses the call and leaves no definition behind. Refusals: a
malformed primitive or ATTDEF (names `entities[i]`/`attdefs[i]` and the
key), a non-finite base point (names `base_x`/`base_y`), a name clash
without `overwrite`, an anonymous `*` name, and a primitive `layer` that
is not in the drawing's layer table (names the missing layers) unless
`create_layers=true`, which creates them and lists them in
`layers_created`. `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)
return await _backend(ctx).block_define(
name, entities, attdefs, base_x, base_y, overwrite, create_layers
)
@cad_tool(summary="Find every place a given block is inserted.", cost="read")
@@ -3926,7 +3988,8 @@ _BATCH_GUARANTEE_NOTES = {
"whole document to a temporary DXF, so it scales with drawing size."
),
"best_effort_undo": (
"Rollback ends the AutoCAD undo mark and sends '_UNDO B' to the command line. "
"Rollback ends the AutoCAD undo group and sends '_UNDO _B' (back to the UNDO "
"Mark transaction_begin set) to the command line. "
"AutoCAD executes that asynchronously and does not confirm it landed, so this "
"is best-effort, NOT atomic. The before/after fingerprint below is the only "
"evidence available; treat a mismatch as 'the undo has not landed (yet)' and "
@@ -4932,7 +4995,8 @@ async def transaction_commit(ctx: Context = None) -> dict:
async def transaction_rollback(ctx: Context = None) -> dict:
"""Rollback the current transaction to the point of transaction_begin.
COM: Undoes all operations back to the last undo mark.
COM: Undoes all operations back to the UNDO Mark transaction_begin set;
refused (`ok: false`) when no transaction is active.
ezdxf: Restores the document from the saved DXF snapshot.
WARNING: This is destructive – all changes since transaction_begin are lost.
@@ -6598,6 +6662,11 @@ async def pid_line_draw(
(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.
A signal line carries no pipe line number: it gets no auto-built number
and no label, and does not consume the process sequence — a verbatim
`line_number` is still written and labelled. With waypoints, a bubble's
exit leaves towards the first waypoint (and the entry arrives from the
last one); `crossings` follows existing arcs, not their chords.
"""
from engineering.pid.drawlines import draw_line
@@ -6775,11 +6844,17 @@ async def pid_from_spec(
) -> 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.
sheet back and the error names it (`lines[2].to`); so does an interruption
(a client cancellation mid-run leaves no half-drawn sheet and no open
transaction on either engine — the live one goes back to the UNDO Mark
`transaction_begin` set, so it never waits at an AutoCAD prompt; should
that rollback itself fail, the failure is logged, the cancellation is what
the client sees, and the drawing must be checked before a retry).
A non-finite coordinate, rotation, scale or stub is refused
by path before anything is placed. 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
+1
View File
@@ -34,6 +34,7 @@ async def test_defines_geometry_and_attdefs(backend):
"entity_count": 7,
"attdef_count": 2,
"replaced": False,
"layers_created": [],
"backend": "ezdxf",
}
blk = backend._doc.blocks.get("PID_VALVE_TEST")
+2 -2
View File
@@ -33,7 +33,7 @@ the happy path is the easy half:
classifier must recover the real cause from the ``__cause__`` chain and
never from the message text.
* **Atomicity honesty.** ezdxf rolls back by restoring a full DXF snapshot;
COM ends an undo mark and fires ``_UNDO B`` at AutoCAD without ever
COM ends an undo group and fires ``_UNDO _B`` at AutoCAD without ever
confirming it landed. The payload must say which one it got, and must not
claim a rollback it did not verify.
* **dry_run.** Validates against each tool's real JSON Schema without
@@ -736,7 +736,7 @@ async def test_a_snapshot_backend_is_reported_as_a_snapshot_guarantee():
async def test_an_undo_mark_backend_is_never_called_atomic():
"""COM ends the undo mark and fires ``_UNDO B`` without confirming it landed."""
"""COM ends the undo group and fires ``_UNDO _B`` without confirming it landed."""
guarantee, note = server._batch_rollback_guarantee(_Caps("undo_mark"))
assert guarantee == "best_effort_undo"
lowered = note.lower()
+71 -6
View File
@@ -101,7 +101,7 @@ async def test_run_lisp_captures_value_from_users1(monkeypatch):
async def test_transaction_begin_sets_flag_and_marks_undo(monkeypatch):
doc = _Doc()
doc.StartUndoMark = MagicMock()
doc.StartUndoMark = MagicMock(side_effect=lambda: doc.sent.append("StartUndoMark"))
monkeypatch.setattr(cb, "_acad_doc", lambda: doc)
b = _backend_no_executor()
@@ -109,6 +109,25 @@ async def test_transaction_begin_sets_flag_and_marks_undo(monkeypatch):
assert out["ok"] is True
assert b._transaction_active is True
doc.StartUndoMark.assert_called_once()
# The UNDO Mark goes in *before* the undo group: `_UNDO _B` in the rollback
# returns to it, and a Mark inside the group would be undone with it.
assert doc.sent == ["_.UNDO _M\n", "StartUndoMark"], doc.sent
async def test_transaction_begin_refuses_while_a_prompt_is_active(monkeypatch):
"""With CMDACTIVE set, SendCommand would feed `_UNDO _M` to the active
command as input and no Mark would exist for the rollback to reach."""
doc = _Doc()
doc.SetVariable("CMDACTIVE", 1)
doc.StartUndoMark = MagicMock()
monkeypatch.setattr(cb, "_acad_doc", lambda: doc)
b = _backend_no_executor()
out = await b.transaction_begin()
assert out["ok"] is False and "CMDACTIVE" in out["error"] and "ESC" in out["error"]
assert b._transaction_active is False, "nothing reached AutoCAD"
assert doc.sent == []
doc.StartUndoMark.assert_not_called()
async def test_transaction_commit_clears_flag_on_error():
@@ -124,17 +143,63 @@ async def test_transaction_commit_clears_flag_on_error():
assert b._transaction_active is False # not left stale
async def test_transaction_rollback_uses_safe_send_command(monkeypatch):
doc = _Doc() # idle: CMDACTIVE=0 so _safe_send_command returns promptly
class _MarkingDoc(_Doc):
"""An idle doc that keeps AutoCAD's UNDO Mark bookkeeping.
On the live AutoCAD 2026 `_UNDO B` with no Mark answers "This will undo
everything. OK? <Y>" and `SendCommand` blocks at that prompt until a human
presses ESC (measured: the rollback hit the 60 s COM timeout, the entities
stayed, every COM client was rejected meanwhile). `StartUndoMark` /
`EndUndoMark` form an undo *group*, not a Mark. This fake answers the
unmarked Back the way the prompt would end up: as a failure, not a hang.
"""
def __init__(self):
super().__init__()
self.marks = 0
self.group_open = False
def SendCommand(self, cmd):
super().SendCommand(cmd)
if cmd.strip() == "_.UNDO _M":
self.marks += 1
elif cmd.strip() == "_.UNDO _B":
if self.marks == 0:
raise RuntimeError("prompt: This will undo everything. OK? <Y>")
self.marks -= 1
def StartUndoMark(self):
self.group_open = True
def EndUndoMark(self):
self.group_open = False
async def test_transaction_rollback_goes_back_to_the_mark_begin_set(monkeypatch):
doc = _MarkingDoc() # idle: CMDACTIVE=0 so _safe_send_command returns promptly
monkeypatch.setattr(cb, "_acad_doc", lambda: doc)
b = _backend_no_executor()
b._transaction_active = True
assert (await b.transaction_begin())["ok"] is True
out = await b.transaction_rollback()
assert out["ok"] is True
assert b._transaction_active is False
# UNDO B went through _safe_send_command (which appends a trailing newline).
assert any("_UNDO B" in c for c in doc.sent)
# UNDO Back went through _safe_send_command (which appends a trailing
# newline) and found the Mark begin set: no prompt, group closed, Mark consumed.
assert [c.strip() for c in doc.sent] == ["_.UNDO _M", "_.UNDO _B"]
assert doc.marks == 0 and doc.group_open is False
async def test_transaction_rollback_without_a_transaction_is_refused(monkeypatch):
"""No transaction means no Mark: an unmarked `_UNDO _B` is the prompt
that strands the STA worker, so it is never sent."""
doc = _MarkingDoc()
monkeypatch.setattr(cb, "_acad_doc", lambda: doc)
b = _backend_no_executor()
out = await b.transaction_rollback()
assert out == {"ok": False, "error": "No active transaction to rollback"}
assert doc.sent == []
# ── R13 — offset honors side_x/side_y and deletes the unused copy ───────────
+7 -1
View File
@@ -167,6 +167,12 @@ async def test_timed_out_transaction_begin_does_not_hide_an_open_undo_mark(
marks = []
class _FakeDoc:
def GetVariable(self, name):
return 0 # CMDACTIVE clear
def SendCommand(self, cmd):
marks.append(cmd.strip()) # the UNDO Mark begin sets first
def StartUndoMark(self):
marks.append("start")
try:
@@ -185,7 +191,7 @@ async def test_timed_out_transaction_begin_does_not_hide_an_open_undo_mark(
)
again = await com_backend.transaction_begin()
assert again == {"ok": False, "error": "A transaction is already active"}
assert marks == ["start"], "the second begin must not nest another undo mark"
assert marks == ["_.UNDO _M", "start"], "the second begin must not nest another undo mark"
finally:
release.set()
assert finished.wait(5)
+36 -10
View File
@@ -119,20 +119,46 @@ async def test_line_list_rows(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"
# An ISA-5.1 signal line carries no pipe line number (track E, Task 6):
# the pneumatic line is unnumbered and did not consume a sequence number,
# so the two process lines are 1 and 2.
assert [r["line_number"] for r in numbered] == ["100-P-1-CS1", "100-P-2-CS1"]
signal = next(r for r in rows if r["line_class"] == "pneumatic")
assert signal["line_number"] is None and signal["number_source"] is None
assert signal["from_tag"] == "FIC-101" and signal["to_tag"] == "FCV-101"
assert signal["to_port"] == "signal"
first = numbered[1]
first = numbered[0]
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
unnumbered = [r for r in rows if r["line_number"] is None]
assert rows[-2:] == unnumbered, "untagged rows sort last"
hand_drawn = next(r for r in unnumbered if r["from_tag"] == "V-201")
assert hand_drawn["from_port"] == "N4"
assert hand_drawn["to_tag"] is None and hand_drawn["number_source"] is None
async def test_line_list_keeps_a_signal_line_unnumbered_beside_a_labelled_pipe(backend):
"""The line list prints what the reader publishes, and the reader must not
hand a 12 mm-parallel signal run the header's label (its own XDATA says
``number: null``)."""
proc = await draw_line(
backend, {"x": 20, "y": 100}, {"x": 200, "y": 100}, size="100", service="P"
)
ft = await place_symbol(
backend, "instrument", 40, 112, tag="FT-1", type="dcs", location="primary"
)
fic = await place_symbol(
backend, "instrument", 180, 112, tag="FIC-1", type="dcs", location="primary"
)
sig = await draw_line(backend, {"handle": ft["handle"]}, {"handle": fic["handle"]}, "electric")
rows = {r["handle"]: r for r in line_list(await build_graph(backend))}
assert (rows[proc["handle"]]["line_number"], rows[proc["handle"]]["number_source"]) == (
"100-P-1",
"xdata",
)
signal = rows[sig["handle"]]
assert (signal["line_number"], signal["number_source"]) == (None, None)
assert (signal["from_tag"], signal["to_tag"]) == ("FT-1", "FIC-1")
async def test_equipment_list_rows(backend):
+344
View File
@@ -85,3 +85,347 @@ async def test_finalize_scores_the_example_at_ninety_or_better(client, tmp_path)
).structured_content
assert final["score"]["score"] >= 90, final["score"]
assert not [i for i in final.get("critique", []) if str(i.get("focus", "")).startswith("pid_")]
# ── Track A hardening (track E wave 0, Task 5) ───────────────────────────────
@pytest.mark.parametrize(
"mutate, fragment",
[
(lambda s: s["equipment"][0].update({"x": float("nan")}), "equipment[0].x"),
(lambda s: s["valves"][0].update({"y": float("inf")}), "valves[0].y"),
(lambda s: s["instruments"][0].update({"rotation": 10**400}), "instruments[0].rotation"),
(lambda s: s["lines"][0].update({"stub": float("-inf")}), "lines[0].stub"),
],
)
async def test_non_finite_numbers_are_refused_by_path_before_any_write(backend, mutate, fragment):
before = await backend.entity_count()
spec = copy.deepcopy(EXAMPLE_SPEC)
mutate(spec)
with pytest.raises(ValueError, match="finite") as exc:
await run_spec(backend, spec)
assert fragment in str(exc.value)
assert await backend.entity_count() == before
assert (await backend.system_status())["transaction_depth"] == 0
async def _wait_until(predicate, *, timeout: float = 5.0) -> None:
"""Poll an async predicate; a rollback that never lands fails here, not by hanging."""
import asyncio
import time
deadline = time.monotonic() + timeout
while not await predicate():
if time.monotonic() > deadline:
raise AssertionError(f"condition not met within {timeout:.1f}s")
await asyncio.sleep(0.02)
@pytest.mark.parametrize("how", ["scope", "native"])
async def test_a_cancellation_mid_run_rolls_the_sheet_back(backend, monkeypatch, how):
"""A *real* cancellation, not a ``CancelledError`` raised from a mock.
The MCP SDK cancels a request through an anyio cancel scope, which is
level-triggered: ``task.cancel()`` is re-issued on every loop iteration
while the task is inside the cancelled scope, so an unshielded rollback
was cancelled at its first ``await`` before its body ran (five symbols
left in an open transaction). A native ``Task.cancel()`` is delivered once
and clears — raising from the mock only ever exercised that easy case.
"""
import asyncio
import anyio
from engineering.pid import spec as spec_module
before = await backend.entity_count()
in_flight = asyncio.Event()
async def slow_draw_line(*args, **kwargs):
in_flight.set()
await asyncio.sleep(30)
monkeypatch.setattr(spec_module, "draw_line", slow_draw_line)
if how == "scope":
async with anyio.create_task_group() as tg:
tg.start_soon(run_spec, backend, EXAMPLE_SPEC)
await in_flight.wait()
assert await backend.entity_count() > before, "symbols placed before the cancel"
tg.cancel_scope.cancel()
else:
task = asyncio.ensure_future(run_spec(backend, EXAMPLE_SPEC))
await in_flight.wait()
assert await backend.entity_count() > before, "symbols placed before the cancel"
task.cancel()
with pytest.raises(asyncio.CancelledError):
await task
assert await backend.entity_count() == before, "the placed symbols were rolled back"
assert (await backend.system_status())["transaction_depth"] == 0, "no open transaction"
async def test_a_client_cancellation_through_the_transport_rolls_the_sheet_back(
client, monkeypatch
):
"""The docstring's promise, measured end to end: ``notifications/cancelled``
from a real ``fastmcp.Client`` mid-run leaves no half-drawn sheet and no
open transaction."""
import asyncio
from fastmcp.exceptions import McpError
from engineering.pid import spec as spec_module
async def slow_draw_line(*args, **kwargs):
await asyncio.sleep(30)
monkeypatch.setattr(spec_module, "draw_line", slow_draw_line)
async def depth() -> int:
return (await client.call_tool("system_status", {})).structured_content["transaction_depth"]
async def entities() -> int:
return (await client.call_tool("analysis_entity_stats", {})).structured_content[
"total_entities"
]
before = await entities()
# The session hands out ids sequentially, so the next one is the tool
# call's — provided nothing else is sent before the call goes out. Yield
# (no requests) until the counter has moved, then start polling.
request_id = client.session._request_id
call = asyncio.ensure_future(client.call_tool("pid_from_spec", {"spec": EXAMPLE_SPEC}))
async def call_sent() -> bool:
return client.session._request_id > request_id
await _wait_until(call_sent)
await _wait_until(lambda: _is_mid_run(depth, entities, before))
await client.cancel(request_id)
with pytest.raises(McpError, match="cancelled"):
await call
# The SDK answers the cancel before the handler's rollback has landed.
await _wait_until(lambda: _is_clean(depth, entities, before))
async def _is_mid_run(depth, entities, before) -> bool:
return await depth() == 1 and await entities() > before
async def _is_clean(depth, entities, before) -> bool:
return await depth() == 0 and await entities() == before
async def test_a_cancellation_with_a_com_call_in_flight_still_closes_the_undo_mark(
monkeypatch,
):
"""COM shape: real ``ComBackend._run``, a real single-thread executor and the
real ``transaction_rollback``; only the ActiveX document is a recorder.
With a COM call blocking the worker when the scope is cancelled, the
unshielded rollback's ``_run`` was cancelled while queued behind it — so
``EndUndoMark`` / ``_UNDO _B`` never reached AutoCAD — yet its ``finally``
cleared ``_transaction_active``: ``system_status`` said "no transaction"
while the undo mark was open, and the next ``transaction_begin`` nested a
second ``StartUndoMark``.
The order pinned here is the one measured on the live AutoCAD 2026: the
UNDO *Mark* goes in before the undo group, so ``_UNDO _B`` lands at the
transaction's start instead of prompting "This will undo everything".
"""
import asyncio
import time
import types
from concurrent.futures import ThreadPoolExecutor
import anyio
from backends import com_backend as module
from engineering.pid import spec as spec_module
sent: list[str] = []
doc = types.SimpleNamespace(
Name="Drawing1.dwg",
FullName="",
GetVariable=lambda name: 0, # CMDACTIVE clear
SendCommand=lambda cmd: sent.append(cmd.strip()),
StartUndoMark=lambda: sent.append("StartUndoMark"),
EndUndoMark=lambda: sent.append("EndUndoMark"),
)
app = types.SimpleNamespace(Documents=types.SimpleNamespace(Count=1), ActiveDocument=doc)
monkeypatch.setattr(module, "_acad_app", lambda: app) # _ensure_document_state
monkeypatch.setattr(module, "_acad_doc", lambda: doc) # begin / rollback
monkeypatch.setattr(
module.ComBackend,
"_safe_send_command",
staticmethod(lambda doc, cmd, deadline_s=8.0: sent.append(cmd) or []),
)
backend = module.ComBackend()
backend._executor = ThreadPoolExecutor(max_workers=1) # no CoInitialize: nothing is COM here
try:
placed = 0
async def fake_place(backend, section, item):
nonlocal placed
placed += 1
return {"handle": f"H{placed}"}
in_flight = asyncio.Event()
async def slow_draw_line(backend, *args, **kwargs):
in_flight.set()
await backend._run(time.sleep, 0.5) # the worker is inside a COM call
monkeypatch.setattr(spec_module, "_place", fake_place)
monkeypatch.setattr(spec_module, "draw_line", slow_draw_line)
async with anyio.create_task_group() as tg:
tg.start_soon(run_spec, backend, EXAMPLE_SPEC)
await in_flight.wait()
tg.cancel_scope.cancel()
assert sent == ["_.UNDO _M", "StartUndoMark", "EndUndoMark", "_.UNDO _B"], sent
assert backend._transaction_active is False
# The mark really is closed, so the next transaction is a fresh one.
assert (await backend.transaction_begin())["ok"] is True
assert sent[-2:] == ["_.UNDO _M", "StartUndoMark"]
finally:
backend._executor.shutdown(wait=True)
# ── a rollback that fails after a cancellation must not kill the session ─────
async def _cancel_mid_run(run, *, in_flight) -> None:
"""Cancel ``run`` through an anyio scope — the SDK's mechanism — once it is
inside ``draw_line``; returns normally only if the cancellation itself is
what left the task (a task group swallows its own cancellation and
re-raises anything else)."""
import anyio
async with anyio.create_task_group() as tg:
tg.start_soon(run)
await in_flight.wait()
tg.cancel_scope.cancel()
async def test_a_rollback_failure_after_a_cancellation_is_logged_not_raised(
backend, monkeypatch, caplog
):
"""Now that the shielded rollback body runs after a real cancellation it
can fail (the COM hard timeout, ``_safe_send_command``'s deadline, an ezdxf
quarantine refusal). That failure must not replace the cancellation: the
SDK has already answered the cancelled request and suppresses only a
cancellation exception — an ordinary error makes it respond a second time
and ``assert not self._completed`` kills the whole session."""
import asyncio
import logging
from engineering.pid import spec as spec_module
in_flight = asyncio.Event()
async def slow_draw_line(*args, **kwargs):
in_flight.set()
await asyncio.sleep(30)
async def failing_rollback():
raise RuntimeError("AutoCAD did not respond within 60s")
monkeypatch.setattr(spec_module, "draw_line", slow_draw_line)
monkeypatch.setattr(backend, "transaction_rollback", failing_rollback)
with caplog.at_level(logging.ERROR, logger="engineering.pid.spec"):
# With the rollback error escaping, the task group would raise it here.
await _cancel_mid_run(lambda: run_spec(backend, EXAMPLE_SPEC), in_flight=in_flight)
messages = [r.getMessage() for r in caplog.records if r.name == "engineering.pid.spec"]
assert any("did not respond within 60s" in m and "half-drawn" in m for m in messages), messages
monkeypatch.undo()
assert (await backend.transaction_rollback())["ok"] is True # the test's own cleanup
async def test_a_rollback_failure_after_an_ordinary_error_still_propagates(backend, monkeypatch):
"""Only a cancellation is special. After a ``ValueError`` the rollback's
own failure is the error the caller gets, with the original chained."""
from engineering.pid import spec as spec_module
async def bad_place(backend, section, item):
raise ValueError("placement refused")
async def failing_rollback():
raise RuntimeError("AutoCAD did not respond within 60s")
monkeypatch.setattr(spec_module, "_place", bad_place)
monkeypatch.setattr(backend, "transaction_rollback", failing_rollback)
with pytest.raises(RuntimeError, match="did not respond") as exc:
await run_spec(backend, EXAMPLE_SPEC)
assert isinstance(exc.value.__context__, ValueError)
assert "equipment[0]: placement refused" in str(exc.value.__context__)
monkeypatch.undo()
assert (await backend.transaction_rollback())["ok"] is True
async def test_a_rollback_failure_after_a_client_cancellation_leaves_the_session_alive(
client, monkeypatch
):
"""The same through the real transport: ``client.cancel(request id)``
mid-run, the rollback raises, and the *next* request is still answered.
Before the fix the server tried to respond twice to the cancelled request,
the lowlevel server's ``assert not self._completed`` escaped its task
group, ``system_status`` was never answered and ``Client.__aexit__``
re-raised the AssertionError."""
import asyncio
from fastmcp.exceptions import McpError
from backends import ezdxf_backend
from engineering.pid import spec as spec_module
async def slow_draw_line(*args, **kwargs):
await asyncio.sleep(30)
real_rollback = ezdxf_backend.EzdxfBackend.transaction_rollback
fail = True
calls: list[str] = []
async def rollback(self):
calls.append("rollback")
if fail:
raise RuntimeError("simulated rollback failure")
return await real_rollback(self)
monkeypatch.setattr(spec_module, "draw_line", slow_draw_line)
monkeypatch.setattr(ezdxf_backend.EzdxfBackend, "transaction_rollback", rollback)
async def depth() -> int:
return (await client.call_tool("system_status", {})).structured_content["transaction_depth"]
request_id = client.session._request_id
call = asyncio.ensure_future(client.call_tool("pid_from_spec", {"spec": EXAMPLE_SPEC}))
async def call_sent() -> bool:
return client.session._request_id > request_id
await _wait_until(call_sent)
await _wait_until(lambda: _is_depth(depth, 1))
await client.cancel(request_id)
with pytest.raises(McpError, match="cancelled"):
await call
async def rolled_back() -> bool:
return bool(calls)
await _wait_until(rolled_back)
# The session is alive: the next request is answered.
assert await asyncio.wait_for(depth(), timeout=5) == 1, "the failed rollback left it open"
fail = False # the test's own cleanup, through the real rollback
assert (await client.call_tool("transaction_rollback", {})).structured_content["ok"] is True
assert await depth() == 0
async def _is_depth(depth, wanted: int) -> bool:
return await depth() == wanted
+65
View File
@@ -289,6 +289,71 @@ async def test_line_number_and_tags_from_nearby_text(backend):
assert edge["line_number"] == '6"-P-1234-CS1' and edge["number_source"] == "label"
async def test_a_signal_line_next_to_a_pipe_label_never_adopts_the_pipes_number(backend):
"""The label search is a fallback for a line with no record of its own.
A transmitter-to-controller run 12 mm above a labelled header (the
ordinary spacing on a dense sheet) is well inside ``label_search``; its
own payload says ``number: null`` and a signal line has no pipe line
number, so the reader must report None — not the header's number under
``number_source="label"``.
"""
proc = await draw_line(
backend, {"x": 20, "y": 100}, {"x": 200, "y": 100}, size="100", service="P"
)
assert proc["line_number"] == "100-P-1"
ft = await place_symbol(
backend, "instrument", 40, 112, tag="FT-1", type="dcs", location="primary"
)
fic = await place_symbol(
backend, "instrument", 180, 112, tag="FIC-1", type="dcs", location="primary"
)
sig = await draw_line(backend, {"handle": ft["handle"]}, {"handle": fic["handle"]}, "electric")
assert sig["line_number"] is None
graph = await build_graph(backend)
by_handle = {e["id"]: e for e in graph["edges"]}
pipe, signal = by_handle[proc["handle"]], by_handle[sig["handle"]]
assert (pipe["line_number"], pipe["number_source"]) == ("100-P-1", "xdata")
assert (signal["line_number"], signal["number_source"]) == (None, None)
assert signal["from"]["node"] == ft["handle"] and signal["to"]["node"] == fic["handle"]
async def test_a_verbatim_number_on_a_signal_line_is_still_read_back(backend):
ft = await place_symbol(
backend, "instrument", 40, 112, tag="FT-1", type="dcs", location="primary"
)
fic = await place_symbol(
backend, "instrument", 180, 112, tag="FIC-1", type="dcs", location="primary"
)
sig = await draw_line(
backend,
{"handle": ft["handle"]},
{"handle": fic["handle"]},
"electric",
line_number="SIG-7",
)
graph = await build_graph(backend)
edge = next(e for e in graph["edges"] if e["id"] == sig["handle"])
assert (edge["line_number"], edge["number_source"]) == ("SIG-7", "xdata")
async def test_a_foreign_signal_line_next_to_a_pipe_label_stays_unnumbered(backend):
"""A layer-classified signal line (no payload) is a signal all the same:
LINE_NUMBER_RE is the pipe grammar, so a match near it is a neighbour's.
The pipe on the same sheet still takes its label."""
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")
await backend.entity_create_line(0, 6, 100, 6, layer="ELECTRICAL-LINE")
await backend.entity_create_line(0, 10, 100, 10, layer="INSTRUMENT-LINE-SIGNAL")
graph = await build_graph(backend)
by_class = {e["line_class"]: e for e in graph["edges"]}
assert set(by_class) == {"process_major", "electric", "signal_unknown"}
pipe = by_class["process_major"]
assert (pipe["line_number"], pipe["number_source"]) == ('6"-P-1234-CS1', "label")
for cls in ("electric", "signal_unknown"):
assert (by_class[cls]["line_number"], by_class[cls]["number_source"]) == (None, None), cls
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")
+236
View File
@@ -290,3 +290,239 @@ async def test_existing_lines_are_seen_on_the_com_engine(monkeypatch):
assert lines[0]["vertices"] == [(100.0, 106.0), (100.0, 128.0), (207.0, 128.0)]
assert lines[0]["payload"]["seq"] == 7
assert lines[0]["payload"]["from"] == {"handle": "2A", "port": "discharge"}
# ── Track A hardening (track E wave 0, Task 6) ───────────────────────────────
async def _bubble_and_valve(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"
)
return cv, fic
async def test_signal_lines_carry_no_fabricated_number_and_do_not_consume_the_sequence(backend):
"""A signal line has no pipe line number: no auto number, no label, and the
process sequence is left alone."""
cv, fic = await _bubble_and_valve(backend)
pump, vessel = await _pump_and_vessel(backend)
signal = await draw_line(
backend, {"handle": fic["handle"]}, {"handle": cv["handle"], "port": "signal"}, "pneumatic"
)
assert signal["line_number"] is None and signal["label_handle"] is None
payload = await read_payload(backend, signal["handle"])
assert payload["number"] is None and payload["seq"] is None
labels = await backend.entity_count(type_filter="TEXT", layer_filter="PROCESS-LINE-TEXT")
assert labels == 0, "no label text was written for the signal line"
process = await draw_line(
backend,
{"handle": pump["handle"], "port": "discharge"},
{"handle": vessel["handle"], "port": "N3"},
size="100",
service="P",
)
assert process["line_number"] == "100-P-1", "the signal line did not consume seq 1"
assert (await read_payload(backend, process["handle"]))["seq"] == 1
# A signal line given size/service still gets no auto number (kind rules).
electric = await draw_line(
backend, {"handle": fic["handle"]}, {"x": 160, "y": 160}, "electric", size="IA", service="S"
)
assert electric["line_number"] is None and electric["label_handle"] is None
# A verbatim number is honoured and labelled, seq stays None.
named = await draw_line(
backend, {"handle": fic["handle"]}, {"x": 40, "y": 160}, "electric", line_number="IA-7"
)
assert named["line_number"] == "IA-7" and named["label_handle"]
assert (await backend.entity_get(named["label_handle"])).properties["text"] == "IA-7"
assert (await read_payload(backend, named["handle"]))["seq"] is None
# The next process line is 2, not 4.
second = await draw_line(
backend,
{"handle": vessel["handle"], "port": "N4"},
{"x": 300, "y": 128},
size="80",
service="P",
)
assert second["line_number"] == "80-P-2"
async def test_a_bubble_exit_aims_at_the_first_waypoint(backend):
"""With waypoints the radial port must leave towards the first waypoint;
aiming at the far end put the exit on the wrong side and the first segment
cut through the bubble (measured: 4.93 mm from the centre of a r=5 bubble)."""
fic = await place_symbol(
backend, "instrument", 100, 160, tag="FIC-1", type="dcs", location="primary"
)
result = await draw_line(
backend,
{"handle": fic["handle"]},
{"x": 100, "y": 100},
"electric",
route_mode=[[130, 160], [130, 100]],
)
assert result["vertices"] == [[105.0, 160.0], [130.0, 160.0], [130.0, 100.0], [100.0, 100.0]]
# Every vertex of the drawn line is on or outside the bubble.
for x, y in result["vertices"]:
assert ((x - 100) ** 2 + (y - 160) ** 2) ** 0.5 >= 5.0 - 1e-9
async def test_a_bubble_entry_aims_at_the_last_waypoint(backend):
fic = await place_symbol(
backend, "instrument", 100, 160, tag="FIC-1", type="dcs", location="primary"
)
result = await draw_line(
backend,
{"x": 100, "y": 100},
{"handle": fic["handle"]},
"electric",
route_mode=[[130, 100], [130, 160]],
)
assert result["vertices"] == [[100.0, 100.0], [130.0, 100.0], [130.0, 160.0], [105.0, 160.0]]
async def test_dry_run_plans_the_same_bubble_exit_as_the_real_run(backend):
from engineering.pid.spec import run_spec
spec = {
"instruments": [
{
"id": "FIC-1",
"type": "dcs",
"location": "primary",
"x": 100,
"y": 160,
"tag": "FIC-1",
}
],
"valves": [
{
"id": "FCV-1",
"symbol": "globe",
"actuator": "diaphragm",
"x": 100,
"y": 100,
"tag": "FCV-1",
}
],
"lines": [
{
"from": "FIC-1",
"to": "FCV-1.signal",
"class": "electric",
"route": [[130, 160], [130, 109]],
}
],
}
planned = await run_spec(backend, spec, dry_run=True)
assert planned["lines"][0]["vertices"][0] == [105.0, 160.0]
drawn = await run_spec(backend, spec)
edge = drawn["graph"]["edges"][0]
assert edge["line_number"] is None, "signal lines from a spec are unnumbered too"
# ── Track A hardening (track E wave 0, Task 7) ───────────────────────────────
async def test_foreign_line_payloads_are_tolerated_not_raised(backend):
"""A hand-edited payload with a non-dict ``from``, a ``None`` ``to`` and a
bool ``seq`` used to escape as ``TypeError: string indices must be
integers``; reader results never raise."""
poly = await backend.entity_create_polyline([[0, 0], [10, 0]], layer="PROCESS-PIPING-MAIN")
await write_payload(
backend,
poly.handle,
{"v": 1, "kind": "line", "seq": True, "from": "garbage", "to": None, "number": 7},
)
other = await backend.entity_create_polyline([[0, 5], [10, 5]], layer="PROCESS-PIPING-MAIN")
await write_payload(
backend,
other.handle,
{"v": 1, "kind": "line", "seq": "9", "from": {"handle": 3, "port": "x"}, "to": 4},
)
result = await draw_line(
backend, {"x": 0, "y": 50}, {"x": 50, "y": 50}, size="100", service="P"
)
assert result["line_number"] == "100-P-1", "neither True nor '9' counts as a sequence number"
assert result["port_reuse"] == []
def test_payload_helpers_accept_only_well_formed_values():
from engineering.pid.drawlines import payload_port_refs, payload_seq
good = {"from": {"handle": "2A", "port": "discharge"}, "to": {"handle": "2B", "port": None}}
assert payload_port_refs(good) == [("2A", "discharge")]
assert payload_port_refs("garbage") == [] and payload_port_refs(None) == []
assert payload_port_refs({"from": "x", "to": 5}) == []
assert payload_seq({"seq": 7}) == 7
assert payload_seq({"seq": True}) is None and payload_seq({"seq": "9"}) is None
assert payload_seq({"seq": 7.0}) is None and payload_seq(None) is None
async def test_crossings_follow_a_bulged_line_not_its_chord(backend):
"""A foreign P&ID line with a semicircular jump (bulge -1 from (0,0) to
(100,0) bows to +Y, apex (50,50)): a vertical utility line at x=30 from
y=20 to y=60 crosses the arc once at y≈45.8 and its chord never — and so
does one at x=50, straight through the apex. The apex is the axis of
symmetry, where a flattened semicircle (an even chord count) puts a
vertex; a vertex hit is a junction to ``segments_cross``, so a chord
chain reported 0 there. The arc is tested exactly, not flattened."""
msp = backend._msp()
msp.add_lwpolyline(
[(0, 0, 0, 0, -1.0), (100, 0, 0, 0, 0)],
format="xyseb",
dxfattribs={"layer": "PROCESS-PIPING-MAIN"},
)
backend._mark_dirty()
lines = await existing_pid_lines(backend)
assert lines[0]["vertices"] == [(0.0, 0.0), (100.0, 0.0)]
assert lines[0]["bulges"] == [-1.0, 0.0]
result = await draw_line(backend, {"x": 30, "y": 20}, {"x": 30, "y": 60}, line_class="utility")
assert result["crossings"] == 1
apex = await draw_line(backend, {"x": 50, "y": 20}, {"x": 50, "y": 60}, line_class="utility")
assert apex["crossings"] == 1, "through the apex — the flattened vertex — is still one crossing"
below = await draw_line(backend, {"x": 70, "y": -20}, {"x": 70, "y": -60}, line_class="utility")
assert below["crossings"] == 0, "the chord side of the arc is empty"
@pytest.mark.parametrize("extrusion", [(0, 0, 1), (0, 0, -1)])
async def test_crossings_count_a_line_through_a_jump_apex(backend, extrusion):
"""The jump a P&ID draws where two lines cross: (45,0)→(55,0) bulge -1, a
5 mm semicircle with its apex at (50,5). A line at x=50 meets the apex
exactly (36 chords of r=50, 12 of r=5: always even, always a vertex on
the axis) and is one crossing, the same as at x=50.3 or x=49 — on a
mirrored frame (extrusion -Z, WCS x negated) too."""
msp = backend._msp()
msp.add_lwpolyline(
[(0, 0, 0, 0, 0), (45, 0, 0, 0, -1.0), (55, 0, 0, 0, 0), (100, 0, 0, 0, 0)],
format="xyseb",
dxfattribs={"layer": "PROCESS-PIPING-MAIN", "extrusion": extrusion},
)
backend._mark_dirty()
sign = extrusion[2]
for x in (50.0, 50.3, 49.0):
drawn = await draw_line(
backend, {"x": sign * x, "y": -20}, {"x": sign * x, "y": 20}, line_class="utility"
)
assert drawn["crossings"] == 1, f"x={sign * x}"
await backend.entity_delete(drawn["handle"])
beside = await draw_line(
backend, {"x": sign * 60, "y": 2}, {"x": sign * 60, "y": 20}, line_class="utility"
)
assert beside["crossings"] == 0, "clear of the jump and above the straight run"
async def test_com_existing_lines_carry_bulges(monkeypatch):
payload = line_payload("process_major", "100-P-7", "100", "P", None, None, 7, None, None)
poly = _com_pid_polyline("3E", (0.0, 0.0, 100.0, 0.0), payload)
poly.Length = 157.0796 # a semicircle: longer than the 100 mm chord walk
poly.GetBulge = lambda index: -1.0 if index == 0 else 0.0
backend = _fake_com(monkeypatch, [poly])
lines = await existing_pid_lines(backend)
assert lines[0]["vertices"] == [(0.0, 0.0), (100.0, 0.0)]
assert lines[0]["bulges"] == [-1.0, 0.0]
+106
View File
@@ -3,6 +3,8 @@
from __future__ import annotations
import math
import pytest
from engineering.pid.lines import (
@@ -176,3 +178,107 @@ def test_route_refuses_non_axis_port_directions_with_a_value_error():
# Equivalent angles and engine float drift still resolve to the axis.
assert route((0, 0), -360.0, (50, 0), 540.0) == [(0.0, 0.0), (50.0, 0.0)]
assert route((0, 0), 1e-9, (50, 0), 180.0 - 1e-9) == [(0.0, 0.0), (50.0, 0.0)]
# ── Track A hardening (track E wave 0, Task 7): bulge flattening ─────────────
@pytest.mark.parametrize("bulge", [1.0, -1.0, 0.5, -0.3, 2.0, 0.2])
def test_flatten_bulges_matches_ezdxf_flattening(bulge):
"""Pinned against ezdxf's own arc flattening: same vertex count, same points."""
from ezdxf.math import ConstructionArc, Vec2, bulge_to_arc
from engineering.pid.lines import FLATTEN_SAGITTA, flatten_bulges
a, b = (0.0, 0.0), (10.0, 0.0)
mine = flatten_bulges([a, b], [bulge])
centre, start_rad, end_rad, radius = bulge_to_arc(Vec2(a), Vec2(b), bulge)
arc = ConstructionArc(
center=centre,
radius=radius,
start_angle=math.degrees(start_rad),
end_angle=math.degrees(end_rad),
)
reference = [(p.x, p.y) for p in arc.flattening(FLATTEN_SAGITTA)]
assert len(mine) == len(reference)
assert mine[0] == a and mine[-1] == b
assert all(math.hypot(p[0] - centre.x, p[1] - centre.y) == pytest.approx(radius) for p in mine)
assert mine[len(mine) // 2] == pytest.approx(reference[len(reference) // 2])
def test_crossings_test_the_exact_arc_so_a_flattened_vertex_hides_nothing():
"""(45,0)→(55,0) bulge -1 is the r=5 jump; its apex (50,5) is a vertex of
the flattened chain (12 chords, even), so the chord chain drops a line at
x=50 from both neighbours. The exact arc counts it once — as it does at
x=50.3 — and keeps the junction rule everywhere it belongs."""
from engineering.pid.lines import count_crossings, flatten_bulges, segment_arc_crossings
jump, bulges = [(45.0, 0.0), (55.0, 0.0)], [-1.0, 0.0]
flat = flatten_bulges(jump, bulges)
assert (50.0, 5.0) in flat, "the apex is a flattened vertex — the case that was missed"
assert count_crossings([(50, -20), (50, 20)], [flat]) == 0, "chords: the hit is a vertex"
assert count_crossings([(50, -20), (50, 20)], [jump], bulges=[bulges]) == 1
assert count_crossings([(50.3, -20), (50.3, 20)], [jump], bulges=[bulges]) == 1
assert count_crossings([(50, 20), (50, 60)], [[(0, 0), (100, 0)]], bulges=[[-1.0, 0.0]]) == 1
# Junctions stay junctions: the segment ending on the arc, a line through
# the arc's own endpoint (the polyline's vertex), the empty chord side,
# and a tangent along the apex that touches without crossing.
assert segment_arc_crossings((50, 5), (50, 20), *jump, -1.0) == 0
assert segment_arc_crossings((45, -5), (45, 5), *jump, -1.0) == 0
assert segment_arc_crossings((50, -20), (50, -1), *jump, -1.0) == 0
assert segment_arc_crossings((40, 5), (60, 5), *jump, -1.0) == 0
assert segment_arc_crossings((40, 4.9), (60, 4.9), *jump, -1.0) == 2, "in and out again"
assert segment_arc_crossings((50, -20), (50, 20), *jump, 1.0) == 1, "bulge +1 bows to -Y"
assert segment_arc_crossings((50, -20), (50, 20), *jump, 0.0) == 1, "bulge 0 is the chord"
# A short or missing bulge list is zero-padded: straight edges as before.
assert count_crossings([(5, -5), (5, 5)], [[(0, 0), (10, 0), (10, 10)]], bulges=[[]]) == 1
assert count_crossings([(5, -5), (5, 5)], [[(0, 0), (10, 0)]], bulges=None) == 1
@pytest.mark.parametrize("seed", range(4))
def test_segment_arc_crossings_match_ezdxf_arc_line_intersection(seed):
"""Pinned against ezdxf's own ``ConstructionArc.intersect_line`` (hits
strictly inside the segment) over random segments and bulges of either
sign, on both sides of a semicircle."""
import random
from ezdxf.math import ConstructionArc, ConstructionLine, Vec2, bulge_to_arc
from engineering.pid.lines import segment_arc_crossings
rng = random.Random(seed)
checked = 0
for _ in range(400):
a = (rng.uniform(-50, 50), rng.uniform(-50, 50))
b = (rng.uniform(-50, 50), rng.uniform(-50, 50))
if math.dist(a, b) < 1.0:
continue
magnitude = rng.choice([0.2, 0.5, 1.0, 1.5, 2.5, rng.uniform(0.05, 3.0)])
bulge = rng.choice([-1.0, 1.0]) * magnitude
p1 = (rng.uniform(-80, 80), rng.uniform(-80, 80))
p2 = (rng.uniform(-80, 80), rng.uniform(-80, 80))
centre, s_rad, e_rad, radius = bulge_to_arc(Vec2(a), Vec2(b), bulge)
arc = ConstructionArc(
center=centre,
radius=radius,
start_angle=math.degrees(s_rad),
end_angle=math.degrees(e_rad),
)
d = Vec2(p2) - Vec2(p1)
hits = arc.intersect_line(ConstructionLine(Vec2(p1), Vec2(p2)), abs_tol=1e-9)
expected = sum(1 for h in hits if 1e-6 < (h - Vec2(p1)).dot(d) / d.dot(d) < 1 - 1e-6)
assert segment_arc_crossings(p1, p2, a, b, bulge) == expected, (p1, p2, a, b, bulge)
checked += 1
assert checked > 350
def test_flatten_bulges_passes_straight_edges_through_and_pads_short_bulge_lists():
from engineering.pid.lines import flatten_bulges
pts = [(0.0, 0.0), (10.0, 0.0), (10.0, 10.0)]
assert flatten_bulges(pts, []) == pts
assert flatten_bulges(pts, [0.0, 0.0, 0.0]) == pts
flat = flatten_bulges(pts, [0.0, 1.0])
assert flat[0] == (0.0, 0.0) and flat[1] == (10.0, 0.0) and flat[-1] == (10.0, 10.0)
assert len(flat) > 3, "the second edge became a semicircle of chords"
assert flatten_bulges([(1.0, 1.0)], [1.0]) == [(1.0, 1.0)]
File diff suppressed because it is too large Load Diff