diff --git a/backends/block_specs.py b/backends/block_specs.py index 3fdc85c..ab4fbaa 100644 --- a/backends/block_specs.py +++ b/backends/block_specs.py @@ -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.""" diff --git a/backends/com_backend.py b/backends/com_backend.py index b0aa28d..fee04bc 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -193,6 +193,70 @@ def _msp(): return doc.ModelSpace +def _int_member(obj, name: str) -> int | None: + """``int(obj.)`` 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? " 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? " 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: diff --git a/backends/contracts/blocks.py b/backends/contracts/blocks.py index bc186d7..2bc4922 100644 --- a/backends/contracts/blocks.py +++ b/backends/contracts/blocks.py @@ -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}``.""" ... diff --git a/backends/ezdxf_backend.py b/backends/ezdxf_backend.py index 465d1d8..ab63bb4 100644 --- a/backends/ezdxf_backend.py +++ b/backends/ezdxf_backend.py @@ -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", } diff --git a/benchmarks/adapters/autocad_mcp_pro.py b/benchmarks/adapters/autocad_mcp_pro.py index 6203458..92ed439 100644 --- a/benchmarks/adapters/autocad_mcp_pro.py +++ b/benchmarks/adapters/autocad_mcp_pro.py @@ -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 diff --git a/discovery/aliases.py b/discovery/aliases.py index e8b9af9..739cda5 100644 --- a/discovery/aliases.py +++ b/discovery/aliases.py @@ -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( diff --git a/docs/analysis/track-a-review-backlog.md b/docs/analysis/track-a-review-backlog.md new file mode 100644 index 0000000..3a1d2cc --- /dev/null +++ b/docs/analysis/track-a-review-backlog.md @@ -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. diff --git a/engineering/pid/drawlines.py b/engineering/pid/drawlines.py index 6761a8b..b635606 100644 --- a/engineering/pid/drawlines.py +++ b/engineering/pid/drawlines.py @@ -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( diff --git a/engineering/pid/graph.py b/engineering/pid/graph.py index 1e52a28..6e570a2 100644 --- a/engineering/pid/graph.py +++ b/engineering/pid/graph.py @@ -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( diff --git a/engineering/pid/lines.py b/engineering/pid/lines.py index abf6b9d..f3d5729 100644 --- a/engineering/pid/lines.py +++ b/engineering/pid/lines.py @@ -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 diff --git a/engineering/pid/spec.py b/engineering/pid/spec.py index 1f5d01b..d8ea20e 100644 --- a/engineering/pid/spec.py +++ b/engineering/pid/spec.py @@ -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) diff --git a/server.py b/server.py index 27a06b2..3f0655f 100644 --- a/server.py +++ b/server.py @@ -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 diff --git a/tests/test_block_define.py b/tests/test_block_define.py index 1756e1b..09e5feb 100644 --- a/tests/test_block_define.py +++ b/tests/test_block_define.py @@ -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") diff --git a/tests/test_cad_batch.py b/tests/test_cad_batch.py index 815ec61..1278d2e 100644 --- a/tests/test_cad_batch.py +++ b/tests/test_cad_batch.py @@ -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() diff --git a/tests/test_com_sprint3.py b/tests/test_com_sprint3.py index 743ff11..a2cd458 100644 --- a/tests/test_com_sprint3.py +++ b/tests/test_com_sprint3.py @@ -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? " 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? ") + 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 ─────────── diff --git a/tests/test_com_timeout.py b/tests/test_com_timeout.py index 46cd542..0368171 100644 --- a/tests/test_com_timeout.py +++ b/tests/test_com_timeout.py @@ -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) diff --git a/tests/test_pid_deliverables.py b/tests/test_pid_deliverables.py index bb28a58..a6c721f 100644 --- a/tests/test_pid_deliverables.py +++ b/tests/test_pid_deliverables.py @@ -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): diff --git a/tests/test_pid_from_spec.py b/tests/test_pid_from_spec.py index d0bc871..df9723b 100644 --- a/tests/test_pid_from_spec.py +++ b/tests/test_pid_from_spec.py @@ -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 diff --git a/tests/test_pid_graph.py b/tests/test_pid_graph.py index 160160d..1786c5d 100644 --- a/tests/test_pid_graph.py +++ b/tests/test_pid_graph.py @@ -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") diff --git a/tests/test_pid_line_draw.py b/tests/test_pid_line_draw.py index 7401307..b8b7f84 100644 --- a/tests/test_pid_line_draw.py +++ b/tests/test_pid_line_draw.py @@ -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] diff --git a/tests/test_pid_lines.py b/tests/test_pid_lines.py index 9ccebe9..2751709 100644 --- a/tests/test_pid_lines.py +++ b/tests/test_pid_lines.py @@ -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)] diff --git a/tests/test_track_a_hardening.py b/tests/test_track_a_hardening.py new file mode 100644 index 0000000..ec145c6 --- /dev/null +++ b/tests/test_track_a_hardening.py @@ -0,0 +1,1951 @@ +"""Track A hardening (v1.6 track E, wave 0): the block-tool defects the Track A +review found, fixed on both engines. + +Headless tests run against a real ezdxf document; the live engine runs against +a fake ActiveX surface that records every call, so the tests assert the call +shape (what is dispatched, in which order, and — for a refusal — that nothing +is dispatched at all). +""" + +from __future__ import annotations + +import types + +import pytest + +from backends.base import EntityInfo + +pytestmark = pytest.mark.asyncio + + +def _define_tagged_block(backend, name: str = "TB") -> None: + blk = backend._doc.blocks.new(name=name) + blk.add_line((0, 0), (10, 0)) + blk.add_attdef("TAG", insert=(0, 3), text="", dxfattribs={"height": 2.5}) + + +# ── fake ActiveX surface ────────────────────────────────────────────────────── + + +#: Members AutoCAD's ActiveX takes as a ``VT_ARRAY|VT_R8`` VARIANT. A plain +#: Python tuple marshals as ``VT_ARRAY|VT_VARIANT`` and is refused with +#: ``E_INVALIDARG`` (verified live on AutoCAD 2026: ``text.Normal = (0, 0, -1)`` +#: raises, ``text.Normal = _apoint(0, 0, -1)`` succeeds); the fake refuses it +#: the same way so a test cannot pin the wrong call shape. +_POINT_MEMBERS = frozenset({"Normal", "InsertionPoint", "TextAlignmentPoint"}) + + +class _FakeObject: + """An ActiveX object with only the members it is given; property writes are recorded.""" + + def __init__(self, object_name: str, handle: str, **members): + object.__setattr__(self, "ObjectName", object_name) + object.__setattr__(self, "Handle", handle) + object.__setattr__(self, "writes", []) + object.__setattr__(self, "deleted", False) + for name, value in members.items(): + object.__setattr__(self, name, value) + + def __setattr__(self, name, value): + if name in _POINT_MEMBERS and not hasattr(value, "varianttype"): + raise RuntimeError( + f"E_INVALIDARG: {name} takes a VT_ARRAY|VT_R8 VARIANT, not {type(value).__name__}" + ) + self.writes.append((name, value)) + object.__setattr__(self, name, value) + + def __getattr__(self, name): + raise AttributeError(name) + + def Delete(self): + object.__setattr__(self, "deleted", True) + + +class _FakeMText(_FakeObject): + """``AddMText``'s MTEXT. ActiveX ``AttachmentPoint`` (and ``Normal``) keep + the text *body* where it is and relocate ``InsertionPoint`` to the new + corner (verified live on AutoCAD 2026: attachment 7 on a two-line note + anchored at (10,10) moved the anchor to (10,-3.33)), so a writer that sets + the anchor first and the attachment second leaves the body TopLeft-at- + anchor. The fake displaces the anchor on those writes -- without recording + a write -- so a test asserts where the anchor *ended up*, not only what + was sent. + """ + + def __setattr__(self, name, value): + super().__setattr__(name, value) + if name in ("AttachmentPoint", "Normal") and hasattr(self, "InsertionPoint"): + x, y, z = self.InsertionPoint.value + moved = types.SimpleNamespace(varianttype="VT_ARRAY|VT_R8", value=[x + 1.0, y - 2.0, z]) + object.__setattr__(self, "InsertionPoint", moved) + + +class _FakeBlock: + def __init__(self, name: str, is_layout: bool = False, is_xref: bool = False): + self.Name = name + self.IsLayout = is_layout + self.IsXRef = is_xref + self.Count = 0 + self.calls: list[tuple[str, tuple]] = [] + + def __getattr__(self, attr): + if attr.startswith("Add"): + + def _call(*args): + self.calls.append((attr, args)) + return _FakeObject("AcDbEntity", f"M{len(self.calls)}") + + return _call + raise AttributeError(attr) + + +class _FakeBlocks: + def __init__(self, *names: str): + self.blocks: dict[str, _FakeBlock] = { + "*Model_Space": _FakeBlock("*Model_Space", is_layout=True), + } + for name in names: + self.blocks[name] = _FakeBlock(name) + + def Add(self, base_point, name): + self.blocks[name] = _FakeBlock(name) + self.blocks[name].base_point = tuple(base_point.value) + return self.blocks[name] + + def Item(self, name): + if name not in self.blocks: + raise RuntimeError(f"no block {name}") + return self.blocks[name] + + +class _FakeLayers: + """The ActiveX layer table: ``Item`` and ``Add`` match names case-insensitively, + and ``Add`` of an existing name hands the existing record back instead of + creating a second one — so ``added`` records only the layers that really + appeared.""" + + def __init__(self, *names: str): + self.names: dict[str, str] = {n.lower(): n for n in ("0", *names)} + self.added: list[str] = [] + + def Item(self, name): + if name.lower() not in self.names: + raise RuntimeError(f"no layer {name}") + return types.SimpleNamespace(Name=self.names[name.lower()]) + + def Add(self, name): + if name.lower() not in self.names: + self.names[name.lower()] = name + self.added.append(name) + return types.SimpleNamespace(Name=self.names[name.lower()]) + + +class _FakeSpace: + """A layout block: records InsertBlock / AddText and hands back styled objects.""" + + def __init__(self, attributes: dict[str, str] | None = None, name: str = "*Model_Space"): + self.calls: list[tuple[str, tuple]] = [] + self.attributes = attributes or {} + self.inserted: list[_FakeObject] = [] + self.texts: list[_FakeObject] = [] + self.Name = name + self.IsLayout = True + + def InsertBlock(self, point, name, sx, sy, sz, rotation): + self.calls.append(("InsertBlock", (tuple(point.value), name, sx, sy, sz, rotation))) + attrs = tuple( + _FakeObject("AcDbAttribute", f"A{i}", TagString=tag, TextString=value) + for i, (tag, value) in enumerate(self.attributes.items()) + ) + ref = _FakeObject( + "AcDbBlockReference", f"R{len(self.inserted)}", Name=name, GetAttributes=lambda: attrs + ) + self.inserted.append(ref) + return ref + + def AddText(self, text, point, height): + self.calls.append(("AddText", (text, tuple(point.value), height))) + obj = _FakeObject("AcDbText", f"T{len(self.calls)}") + self.texts.append(obj) + return obj + + def AddMText(self, point, width, text): + self.calls.append(("AddMText", (tuple(point.value), width, text))) + obj = _FakeMText("AcDbMText", f"T{len(self.calls)}", InsertionPoint=point) + self.texts.append(obj) + return obj + + +@pytest.fixture +def com(monkeypatch): + """A ComBackend over a recording fake: ``document`` and ``space`` are inspectable.""" + from backends import com_backend as module + + document = types.SimpleNamespace( + Blocks=_FakeBlocks("TB"), Layers=_FakeLayers(), objects={}, owners={} + ) + document.HandleToObject = lambda handle: document.objects[handle] + # ``OwnerID`` -> block table record, the way ``ObjectIdToObject`` resolves it live. + document.ObjectIdToObject = lambda object_id: document.owners[object_id] + space = _FakeSpace() + document.owners[1] = space # the active layout; a sheet registers itself under 2 + monkeypatch.setattr(module, "_acad_doc", lambda: document) + monkeypatch.setattr(module, "_msp", lambda: space) + monkeypatch.setattr(module, "_regen", lambda: None) + # The real converter reads a dozen ActiveX members (bounding box, block + # geometry, ...); the fixture only needs the handle and the name back. + monkeypatch.setattr( + module, + "_entity_info", + lambda ent: EntityInfo( + handle=ent.Handle, + type="INSERT", + layer="0", + color=256, + linetype="ByLayer", + visible=True, + properties={"block_name": ent.Name}, + ), + ) + backend = module.ComBackend() + + async def _run_inline(func, *args, **kwargs): + return func(*args, **kwargs) + + monkeypatch.setattr(backend, "_run", _run_inline) + return backend, document, space + + +# ── Task 1: entity_create_block_ref refuses an undefined block ──────────────── + + +async def test_block_ref_of_an_undefined_block_is_refused_before_writing(backend): + before = len(list(backend._msp())) + with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"): + await backend.entity_create_block_ref("NO_SUCH_BLOCK", 1, 2) + assert len(list(backend._msp())) == before, "a refused insert must not write" + assert "NO_SUCH_BLOCK" not in backend._doc.blocks + + +async def test_block_ref_of_a_defined_block_still_inserts(backend): + _define_tagged_block(backend) + ref = await backend.entity_create_block_ref("TB", 5, 5, rotation=90) + info = await backend.entity_get(ref.handle) + assert info.type == "INSERT" and info.properties["block_name"] == "TB" + assert info.properties["rotation_deg"] == pytest.approx(90.0) + + +@pytest.mark.parametrize("name", ["*Model_Space", "*Paper_Space"]) +async def test_layout_blocks_are_refused_by_both_placing_tools(backend, name): + with pytest.raises(ValueError, match="layout block"): + await backend.entity_create_block_ref(name, 0, 0) + with pytest.raises(ValueError, match="layout block"): + await backend.block_insert(name, 0, 0) + assert backend._doc.audit().errors == [], "nothing was written, nothing to fix" + + +async def test_a_non_string_block_name_is_a_type_error_on_both_placing_tools(backend): + with pytest.raises(TypeError, match="non-empty string"): + await backend.entity_create_block_ref(None, 0, 0) + with pytest.raises(TypeError, match="non-empty string"): + await backend.block_insert("", 0, 0) + + +async def test_com_block_ref_probes_the_block_table_before_insert_block(com): + backend, document, space = com + with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"): + await backend.entity_create_block_ref("NO_SUCH_BLOCK", 1, 2) + assert space.calls == [], "InsertBlock must not be dispatched for an unknown name" + with pytest.raises(ValueError, match="layout block"): + await backend.entity_create_block_ref("*Model_Space", 0, 0) + assert space.calls == [] + + ref = await backend.entity_create_block_ref("TB", 1, 2, 2.0, 2.0, 90.0) + assert ref.properties["block_name"] == "TB" + assert space.calls == [ + ("InsertBlock", ((1.0, 2.0, 0.0), "TB", 2.0, 2.0, 1.0, pytest.approx(1.5708, abs=1e-4))) + ] + + +async def test_com_block_insert_shares_the_same_gate(com): + backend, document, space = com + with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"): + await backend.block_insert("NO_SUCH_BLOCK", 1, 2, attributes={"TAG": "P-1"}) + assert space.calls == [] + + +# ── Task 2: block_explode keeps ATTRIB text ────────────────────────────────── + + +async def test_explode_emits_a_text_per_attrib_at_the_attribs_placement(backend): + from ezdxf.enums import TextEntityAlignment + + blk = backend._doc.blocks.new(name="TB2") + blk.add_line((0, 0), (10, 0)) + tag = blk.add_attdef("TAG", insert=(0, 3), text="", dxfattribs={"height": 2.5}) + tag.set_placement((0, 3), align=TextEntityAlignment.CENTER) + hidden = blk.add_attdef("LINK", insert=(0, -3), text="L-1", dxfattribs={"height": 2.0}) + hidden.is_invisible = True + ref = await backend.block_insert( + "TB2", 10, 10, scale_x=2.0, scale_y=2.0, rotation=30.0, attributes={"TAG": "P-101"} + ) + raw_ref = backend._doc.entitydb[ref.handle] + expected = {a.dxf.tag: (tuple(a.dxf.insert)[:2], a.dxf.height) for a in raw_ref.attribs} + + result = await backend.block_explode(ref.handle) + + assert result["ok"] is True and result["exploded_handle"] == ref.handle + assert result["backend"] == "ezdxf" + assert [(await backend.entity_get(h)).type for h in result["inserted_handles"]] == ["LINE"] + assert len(result["attribute_texts"]) == 2 + texts = {} + for handle in result["attribute_texts"]: + info = await backend.entity_get(handle) + assert info.type == "TEXT" + texts[info.properties["text"]] = info + tag_text = texts["P-101"] + assert tuple(tag_text.properties["insertion"]) == pytest.approx(expected["TAG"][0]) + assert tag_text.properties["height"] == expected["TAG"][1] == 5.0 + assert tag_text.properties["rotation"] == pytest.approx(30.0) + raw = backend._doc.entitydb[tag_text.handle] + assert raw.get_align_enum().name == "CENTER", "alignment survives the conversion" + assert raw.dxf.invisible == 0 + assert backend._doc.entitydb[texts["L-1"].handle].dxf.invisible == 1 + with pytest.raises(RuntimeError, match="not found"): + await backend.entity_get(ref.handle) + assert backend._doc.audit().errors == [] + + +async def test_explode_without_attribs_reports_an_empty_list(backend): + line = await backend.entity_create_line(0, 0, 10, 0) + await backend.block_create_from_entities("PLAIN", [line.handle]) + ref = await backend.block_insert("PLAIN", 0, 0) + result = await backend.block_explode(ref.handle) + assert result["attribute_texts"] == [] and len(result["inserted_handles"]) == 1 + + +async def test_explode_refuses_a_non_insert_without_writing(backend): + line = await backend.entity_create_line(0, 0, 10, 0) + before = len(list(backend._msp())) + with pytest.raises(RuntimeError, match="not a block reference"): + await backend.block_explode(line.handle) + assert len(list(backend._msp())) == before + + +async def test_com_explode_deletes_attdefs_adds_text_per_attrib_and_deletes_the_reference(com): + backend, document, space = com + attdef = _FakeObject("AcDbAttributeDefinition", "D1", Constant=False) + line = _FakeObject("AcDbLine", "L1") + visible = _FakeObject( + "AcDbAttribute", + "A1", + TagString="TAG", + TextString="P-101", + InsertionPoint=(7.0, 15.2, 0.0), + Height=5.0, + Rotation=0.5236, + Layer="PID-TAG", + Invisible=False, + ) + hidden = _FakeObject( + "AcDbAttribute", + "A2", + TagString="LINK", + TextString="L-1", + InsertionPoint=(13.0, 4.8, 0.0), + Height=4.0, + Rotation=0.5236, + Layer="0", + Invisible=True, + ) + ref = _FakeObject( + "AcDbBlockReference", + "2F", + Name="TB", + OwnerID=1, + GetAttributes=lambda: (visible, hidden), + GetConstantAttributes=lambda: (), + Explode=lambda: (line, attdef), + ) + document.objects["2F"] = ref + + result = await backend.block_explode("2F") + + assert result == { + "ok": True, + "exploded_handle": "2F", + "inserted_handles": ["L1"], + "attribute_texts": ["T1", "T2"], + "backend": "com", + } + assert attdef.deleted is True, "the value-less ATTDEF placeholder is removed" + assert line.deleted is False + assert ref.deleted is True, "ActiveX Explode leaves the reference; BURST removes it" + assert [c for c in space.calls] == [ + ("AddText", ("P-101", (7.0, 15.2, 0.0), 5.0)), + ("AddText", ("L-1", (13.0, 4.8, 0.0), 4.0)), + ] + + +async def test_com_explode_refuses_a_non_insert_before_any_call(com): + backend, document, space = com + document.objects["3A"] = _FakeObject("AcDbLine", "3A") + with pytest.raises(RuntimeError, match="not a block reference"): + await backend.block_explode("3A") + assert space.calls == [] + + +async def test_com_explode_adds_the_text_to_the_owner_block_not_the_active_layout(com): + """ActiveX ``Explode()`` places the members in the owner space; the tag + TEXT must land beside them, not in whatever tab is active.""" + backend, document, space = com + sheet = _FakeSpace(name="Sheet") + document.owners[2] = sheet + title = _FakeObject( + "AcDbAttribute", + "A1", + TagString="TITLE", + TextString="GEAR-01", + InsertionPoint=(100.0, 50.0, 0.0), + Height=3.5, + Rotation=0.0, + Layer="TITLEBLOCK", + Invisible=False, + ) + line = _FakeObject("AcDbLine", "L1") + ref = _FakeObject( + "AcDbBlockReference", + "4B", + Name="TB", + OwnerID=2, + GetAttributes=lambda: (title,), + GetConstantAttributes=lambda: (), + Explode=lambda: (line,), + ) + document.objects["4B"] = ref + + result = await backend.block_explode("4B") + + assert result["ok"] is True and result["inserted_handles"] == ["L1"] + assert sheet.calls == [("AddText", ("GEAR-01", (100.0, 50.0, 0.0), 3.5))] + assert space.calls == [], "nothing may be written into the active layout" + assert ref.deleted is True + + +async def test_com_explode_refuses_a_nested_reference_before_explode(com): + backend, document, space = com + document.owners[3] = _FakeBlock("OUTER") # a block definition, IsLayout False + dispatched = [] + + def _explode(): + dispatched.append("Explode") + return () + + ref = _FakeObject( + "AcDbBlockReference", + "5C", + Name="TB", + OwnerID=3, + GetAttributes=lambda: (), + GetConstantAttributes=lambda: (), + Explode=_explode, + ) + document.objects["5C"] = ref + with pytest.raises(RuntimeError, match="nested inside block definition 'OUTER'"): + await backend.block_explode("5C") + assert dispatched == [] and space.calls == [] and ref.deleted is False + + +async def test_com_explode_carries_the_attrib_frame_and_style_onto_the_text(com): + """The COM twin of the mirrored-reference test. ``AddText`` builds a +Z + TEXT; an ATTRIB reflected to ``Normal (0, 0, -1)`` has an OCS ``Rotation`` + and mirrored glyphs, so the TEXT must take the frame *before* the angle + and have its WCS anchor re-asserted after the frame changes it. The + style members ezdxf carries (style, oblique, width, generation flags, + alignment + alignment point, colour) ride along the same way.""" + backend, document, space = com + attrib = _FakeObject( + "AcDbAttribute", + "A1", + TagString="TAG", + TextString="P-101", + InsertionPoint=(-25.0, 13.0, 0.0), + Height=2.5, + Rotation=0.5236, + Layer="PID-TAG", + Invisible=False, + Normal=(0.0, 0.0, -1.0), + Thickness=0.7, + StyleName="ISO", + ObliqueAngle=0.2618, + ScaleFactor=0.8, + Backward=True, + UpsideDown=False, + Alignment=4, + TextAlignmentPoint=(-20.0, 13.0, 0.0), + Color=3, + ) + ref = _FakeObject( + "AcDbBlockReference", + "6D", + Name="TB", + OwnerID=1, + GetAttributes=lambda: (attrib,), + GetConstantAttributes=lambda: (), + Explode=lambda: (), + ) + document.objects["6D"] = ref + + result = await backend.block_explode("6D") + + assert result["ok"] is True and result["attribute_texts"] == ["T1"] + assert space.calls == [("AddText", ("P-101", (-25.0, 13.0, 0.0), 2.5))] + text = space.texts[0] + # Point writes go through ``_apoint`` (a VARIANT); compare their payload. + import pythoncom + + for name, value in text.writes: + if name in _POINT_MEMBERS: + assert value.varianttype == pythoncom.VT_ARRAY | pythoncom.VT_R8, ( + f"{name} must be a double-array VARIANT, not a tuple (E_INVALIDARG live)" + ) + writes = [ + (name, tuple(value.value) if hasattr(value, "value") else value) + for name, value in text.writes + ] + names = [name for name, _ in writes] + assert ("Normal", (0.0, 0.0, -1.0)) in writes, "the ATTRIB's frame is carried" + assert names.index("Normal") < names.index("Rotation"), ( + "Rotation is the OCS angle: it only means the same thing inside the same frame" + ) + assert names.index("Normal") < names.index("InsertionPoint"), ( + "the WCS anchor is re-asserted after the frame change moves the OCS origin" + ) + assert names.index("Alignment") < names.index("TextAlignmentPoint"), ( + "ActiveX rejects an alignment point on a left-aligned text" + ) + assert dict(writes) == { + "Normal": (0.0, 0.0, -1.0), + "Thickness": 0.7, + "StyleName": "ISO", + "ObliqueAngle": 0.2618, + "ScaleFactor": 0.8, + "Backward": True, + "UpsideDown": False, + "Rotation": 0.5236, + "InsertionPoint": (-25.0, 13.0, 0.0), + "Alignment": 4, + "TextAlignmentPoint": (-20.0, 13.0, 0.0), + "Layer": "PID-TAG", + "Color": 3, + } + + +async def test_com_explode_skips_frame_members_the_attrib_does_not_expose(com): + """A bare ATTRIB (the shape the earlier fakes use) still explodes: an + absent optional member is skipped, never written as a guess, and a + left-aligned ATTRIB never gets a TextAlignmentPoint.""" + backend, document, space = com + attrib = _FakeObject( + "AcDbAttribute", + "A1", + TagString="TAG", + TextString="P-101", + InsertionPoint=(7.0, 15.2, 0.0), + Height=5.0, + Rotation=0.0, + Layer="0", + Invisible=False, + Alignment=0, + TextAlignmentPoint=(99.0, 99.0, 0.0), + ) + ref = _FakeObject( + "AcDbBlockReference", + "7E", + Name="TB", + OwnerID=1, + GetAttributes=lambda: (attrib,), + GetConstantAttributes=lambda: (), + Explode=lambda: (), + ) + document.objects["7E"] = ref + + result = await backend.block_explode("7E") + + assert result["ok"] is True + names = [name for name, _ in space.texts[0].writes] + assert "Normal" not in names and "StyleName" not in names + assert "InsertionPoint" not in names, "a +Z text keeps the anchor AddText was given" + assert "TextAlignmentPoint" not in names, "left alignment has no alignment point" + assert names == ["Rotation", "Alignment", "Layer"] + + +async def test_com_explode_turns_a_constant_attdef_into_text_instead_of_deleting_it(com): + """``GetAttributes()`` excludes constant attributes (``GetConstantAttributes()`` + lists them) and ``Explode()`` returns each as an ``AcDbAttributeDefinition`` + already at its WCS placement showing its value. Deleting every ATTDEF + destroyed that text with ``ok: True``; BURST converts it to TEXT.""" + backend, document, space = com + tag = _FakeObject( + "AcDbAttribute", + "A1", + TagString="TAG", + TextString="P-101", + InsertionPoint=(7.0, 15.2, 0.0), + Height=5.0, + Rotation=0.0, + Layer="PID-TAG", + Invisible=False, + ) + placeholder = _FakeObject("AcDbAttributeDefinition", "D1", TagString="TAG", Constant=False) + constant = _FakeObject( + "AcDbAttributeDefinition", + "D2", + TagString="CONST", + TextString="ACME", + Constant=True, + InsertionPoint=(25.0, 7.0, 0.0), + Height=2.0, + Rotation=0.5236, + Layer="TITLEBLOCK", + Invisible=False, + Alignment=4, + TextAlignmentPoint=(25.0, 7.0, 0.0), + ) + line = _FakeObject("AcDbLine", "L1") + ref = _FakeObject( + "AcDbBlockReference", + "8F", + Name="TB", + OwnerID=1, + GetAttributes=lambda: (tag,), + GetConstantAttributes=lambda: (constant,), + Explode=lambda: (line, placeholder, constant), + ) + document.objects["8F"] = ref + + result = await backend.block_explode("8F") + + assert result["inserted_handles"] == ["L1"] + assert result["attribute_texts"] == ["T1", "T2"], "the constant value is a TEXT too" + assert placeholder.deleted is True and constant.deleted is True and ref.deleted is True + assert space.calls == [ + ("AddText", ("P-101", (7.0, 15.2, 0.0), 5.0)), + ("AddText", ("ACME", (25.0, 7.0, 0.0), 2.0)), + ] + writes = [ + (name, tuple(value.value) if hasattr(value, "value") else value) + for name, value in space.texts[1].writes + ] + assert writes == [ + ("Rotation", 0.5236), + ("Alignment", 4), + ("TextAlignmentPoint", (25.0, 7.0, 0.0)), + ("Layer", "TITLEBLOCK"), + ], "the constant ATTDEF's placement, angle and alignment ride onto its TEXT" + + +async def test_com_explode_falls_back_to_the_constant_tag_list_when_constant_is_absent(com): + """An object that does not expose ``Constant`` is still recognised through + ``GetConstantAttributes()``; a value-less placeholder is still just deleted.""" + backend, document, space = com + constant = _FakeObject( + "AcDbAttributeDefinition", + "D2", + TagString="CONST", + TextString="ACME", + InsertionPoint=(1.0, 2.0, 0.0), + Height=2.0, + Rotation=0.0, + Layer="0", + Invisible=False, + ) + placeholder = _FakeObject("AcDbAttributeDefinition", "D1", TagString="TAG") + ref = _FakeObject( + "AcDbBlockReference", + "9A", + Name="TB", + OwnerID=1, + GetAttributes=lambda: (), + GetConstantAttributes=lambda: (constant,), + Explode=lambda: (placeholder, constant), + ) + document.objects["9A"] = ref + + result = await backend.block_explode("9A") + + assert result["attribute_texts"] == ["T1"] + assert space.calls == [("AddText", ("ACME", (1.0, 2.0, 0.0), 2.0))] + assert placeholder.deleted is True and constant.deleted is True + + +def _define_block_with_constant_attdef(backend, name: str = "TBC") -> None: + from ezdxf.enums import TextEntityAlignment + + blk = backend._doc.blocks.new(name=name) + blk.add_line((0, 0), (10, 0)) + blk.add_attdef("TAG", insert=(0, 3), text="", dxfattribs={"height": 2.5}) + const = blk.add_attdef("CONST", insert=(5, -3), text="ACME", dxfattribs={"height": 2.0}) + const.set_placement((5, -3), align=TextEntityAlignment.MIDDLE_CENTER) + const.dxf.flags |= 2 # ATTRIB_CONST + assert const.is_const + + +async def test_explode_keeps_a_constant_attdef_that_has_no_attrib_as_text(backend): + """The AutoCAD-authored shape: a constant attribute has no ATTRIB on the + reference, and ``virtual_entities()`` skips ATTDEF, so its text used to + vanish with a clean audit. The TEXT lands where ``add_auto_attribs`` + would have put the ATTRIB (mirrored + rotated reference).""" + _define_block_with_constant_attdef(backend) + ref = backend._msp().add_blockref( + "TBC", (20, 10), dxfattribs={"xscale": -1.0, "yscale": 1.0, "rotation": 30.0} + ) + ref.add_auto_attribs({"TAG": "P-101"}) + expected = {a.dxf.tag: a.dxfattribs() for a in ref.attribs}["CONST"] + ref.delete_attrib("CONST") # AutoCAD writes no ATTRIB for a constant attribute + assert [a.dxf.tag for a in ref.attribs] == ["TAG"], "the premise" + + result = await backend.block_explode(ref.dxf.handle) + + assert [(await backend.entity_get(h)).type for h in result["inserted_handles"]] == ["LINE"] + texts = {} + for handle in result["attribute_texts"]: + raw = backend._doc.entitydb[handle] + assert raw.dxftype() == "TEXT" + texts[raw.dxf.text] = raw + assert set(texts) == {"P-101", "ACME"} + acme = texts["ACME"] + for key in ("insert", "align_point", "extrusion"): + assert tuple(acme.dxf.get(key)) == pytest.approx(tuple(expected[key])), key + assert acme.dxf.rotation == pytest.approx(expected["rotation"]) + assert acme.dxf.height == expected["height"] == 2.0 + assert acme.get_align_enum().name == "MIDDLE_CENTER" + assert acme.dxf.invisible == 0 + assert backend._doc.audit().errors == [] + + +async def test_explode_does_not_duplicate_a_constant_attdef_that_already_has_an_attrib(backend): + """The MCP-authored shape: ``block_insert`` attaches an ATTRIB for the + constant ATTDEF too, so it is converted once, not once per source.""" + _define_block_with_constant_attdef(backend) + ref = await backend.block_insert("TBC", 20, 10, attributes={"TAG": "P-101"}) + raw_ref = backend._doc.entitydb[ref.handle] + assert sorted(a.dxf.tag for a in raw_ref.attribs) == ["CONST", "TAG"], "the premise" + + result = await backend.block_explode(ref.handle) + + values = sorted(backend._doc.entitydb[h].dxf.text for h in result["attribute_texts"]) + assert values == ["ACME", "P-101"] + assert backend._doc.audit().errors == [] + + +async def test_explode_of_a_mirrored_reference_keeps_the_tag_where_the_attrib_was(backend): + """An ATTRIB is an OCS entity. Mirroring reflects it to extrusion + ``(0, 0, -1)`` while the INSERT keeps +Z with ``yscale=-1``; the TEXT + that replaces it must carry the frame, or it lands on the other side + of the mirror axis with a clean audit.""" + from backends.ocs import to_wcs_2d + + blk = backend._doc.blocks.new(name="SYM") + blk.add_line((0, 0), (10, 0)) + blk.add_attdef("TAG", insert=(5, 3), text="", dxfattribs={"height": 2.5}) + ref = await backend.block_insert("SYM", 20, 10, attributes={"TAG": "P-101"}) + mirrored = await backend.entity_mirror(ref.handle, 0, 0, 0, 1, delete_original=True) + raw = backend._doc.entitydb[mirrored.handle] + (attrib,) = raw.attribs + assert tuple(attrib.dxf.extrusion) == (0.0, 0.0, -1.0), "the premise: a reflected ATTRIB" + expected = to_wcs_2d(attrib.dxf.extrusion, *attrib.dxf.insert) + assert expected == pytest.approx([-25.0, 13.0]) + + result = await backend.block_explode(mirrored.handle) + + (line_handle,) = result["inserted_handles"] + (text_handle,) = result["attribute_texts"] + line = await backend.entity_get(line_handle) + text = await backend.entity_get(text_handle) + assert text.properties["insertion"] == pytest.approx(expected) + assert text.properties["insertion"][0] < 0 and line.properties["start"][0] < 0, ( + "tag and geometry stay on the same side of the mirror axis" + ) + assert backend._doc.audit().errors == [] + + +async def test_explode_writes_into_the_inserts_owner_layout_not_the_current_one(backend): + _define_tagged_block(backend) + await backend.layout_create("Sheet") + await backend.layout_set_current("Sheet") + ref = await backend.block_insert("TB", 100, 50, attributes={"TAG": "GEAR-01"}) + await backend.layout_set_current("Model") + model_before = len(list(backend._doc.modelspace())) + + result = await backend.block_explode(ref.handle) + + assert result["ok"] is True + sheet = backend._doc.layouts.get("Sheet") + assert sorted(e.dxftype() for e in sheet) == ["LINE", "TEXT"] + assert len(list(backend._doc.modelspace())) == model_before, "model space untouched" + for handle in result["inserted_handles"] + result["attribute_texts"]: + assert backend._doc.entitydb[handle].get_layout().name == "Sheet" + with pytest.raises(RuntimeError, match="not found"): + await backend.entity_get(ref.handle) + assert backend._doc.audit().errors == [] + + +async def test_explode_refuses_a_nested_reference_before_writing(backend): + _define_tagged_block(backend) + outer = backend._doc.blocks.new(name="OUTER") + nested = outer.add_blockref("TB", (0, 0)) + before = (len(list(backend._doc.modelspace())), len(list(outer))) + with pytest.raises(RuntimeError, match="nested inside block definition 'OUTER'"): + await backend.block_explode(nested.dxf.handle) + assert (len(list(backend._doc.modelspace())), len(list(outer))) == before + assert backend._doc.entitydb[nested.dxf.handle].is_alive + + +# ── Task 2 (review): multi-line attributes, xrefs and MINSERTs ─────────────── + + +def _embed_multi_line(attrib, lines: list[str], insert, char_height: float = 2.5): + """Give ``attrib`` (ATTRIB or ATTDEF) an embedded MTEXT the way ezdxf's + ``embed_mtext`` does; ``dxf.text`` then holds only the first line.""" + from ezdxf.entities import MText + + mtext = MText.new(dxfattribs={"char_height": char_height, "insert": insert}) + mtext.text = "\\P".join(lines) + attrib.embed_mtext(mtext) + assert attrib.has_embedded_mtext_entity and attrib.dxf.text == lines[0], "the premise" + + +async def test_explode_keeps_a_multi_line_attrib_as_mtext_with_every_line(backend): + """A multi-line ATTRIB carries its content in an embedded MTEXT; ``dxf.text`` + is only the first line (ezdxf) or ``Line1\\PLine2`` (AutoCAD), so a TEXT + either drops the other lines or renders literal ``\\P``. BURST emits an + MTEXT for a multi-line attribute.""" + blk = backend._doc.blocks.new(name="MLB") + blk.add_line((0, 0), (10, 0)) + blk.add_attdef("NOTE", insert=(0, 3), text="x", dxfattribs={"height": 2.5}) + blk.add_attdef("HIDDEN", insert=(0, -3), text="x", dxfattribs={"height": 2.0}) + ref = await backend.block_insert( + "MLB", + 10, + 10, + scale_x=2.0, + scale_y=2.0, + rotation=30.0, + attributes={"NOTE": "line one", "HIDDEN": "h"}, + ) + raw_ref = backend._doc.entitydb[ref.handle] + attribs = {a.dxf.tag: a for a in raw_ref.attribs} + lines = ["LINE ONE", "LINE TWO", "LINE THREE"] + _embed_multi_line(attribs["NOTE"], lines, attribs["NOTE"].dxf.insert, char_height=5.0) + attribs["NOTE"].dxf.layer = "PID-TAG" + _embed_multi_line(attribs["HIDDEN"], ["H1", "H2"], attribs["HIDDEN"].dxf.insert) + attribs["HIDDEN"].is_invisible = True + expected = attribs["NOTE"].virtual_mtext_entity() + + result = await backend.block_explode(ref.handle) + + assert [(await backend.entity_get(h)).type for h in result["inserted_handles"]] == ["LINE"] + assert len(result["attribute_texts"]) == 2 + by_text = {} + for handle in result["attribute_texts"]: + raw = backend._doc.entitydb[handle] + assert raw.dxftype() == "MTEXT", "a multi-line attribute bursts into an MTEXT" + by_text[raw.text] = raw + note = by_text["\\P".join(lines)] + assert note.plain_text(split=True) == lines + assert tuple(note.dxf.insert) == pytest.approx(tuple(expected.dxf.insert)) + assert note.dxf.char_height == expected.dxf.char_height == 5.0 + assert note.dxf.attachment_point == expected.dxf.attachment_point + assert note.dxf.layer == "PID-TAG", "the ATTRIB's graphic properties ride along" + assert note.dxf.invisible == 0 + assert note.get_layout() is backend._doc.modelspace() + assert by_text["H1\\PH2"].dxf.invisible == 1, "an invisible attribute stays invisible" + with pytest.raises(RuntimeError, match="not found"): + await backend.entity_get(ref.handle) + assert backend._doc.audit().errors == [] + + +async def test_explode_keeps_a_multi_line_constant_attdef_as_mtext(backend): + """The constant twin: no ATTRIB on the reference, the value lives in the + definition's ATTDEF, and that ATTDEF is multi-line.""" + blk = backend._doc.blocks.new(name="MLC") + blk.add_line((0, 0), (10, 0)) + const = blk.add_attdef("CONST", insert=(5, -3), text="x", dxfattribs={"height": 2.0}) + _embed_multi_line(const, ["ACME", "LTD"], (5, -3, 0), char_height=2.0) + const.dxf.flags |= 2 # ATTRIB_CONST + ref = backend._msp().add_blockref("MLC", (20, 10), dxfattribs={"rotation": 90.0}) + assert list(ref.attribs) == [], "the premise: AutoCAD writes no ATTRIB for a constant" + expected = const.virtual_mtext_entity().transform(ref.matrix44()) + + result = await backend.block_explode(ref.dxf.handle) + + (handle,) = result["attribute_texts"] + raw = backend._doc.entitydb[handle] + assert raw.dxftype() == "MTEXT" + assert raw.plain_text(split=True) == ["ACME", "LTD"] + assert tuple(raw.dxf.insert) == pytest.approx(tuple(expected.dxf.insert)) + assert raw.get_rotation() == pytest.approx(90.0) + assert backend._doc.audit().errors == [] + + +async def test_explode_refuses_an_xref_before_writing(backend): + """``virtual_entities()`` of an xref INSERT yields nothing (the geometry is + in the other file), so exploding it deleted the reference with ``ok: True`` + and an empty ``inserted_handles``. AutoCAD refuses to explode an xref.""" + backend._doc.add_xref_def("C:/x/detail.dwg", "XREF_DETAIL") + xref = backend._msp().add_blockref("XREF_DETAIL", (5, 5)) + before = len(list(backend._msp())) + with pytest.raises(RuntimeError, match="external reference 'XREF_DETAIL'"): + await backend.block_explode(xref.dxf.handle) + assert len(list(backend._msp())) == before + assert backend._doc.entitydb[xref.dxf.handle].is_alive + + +async def test_explode_refuses_a_minsert_before_writing(backend): + """``virtual_entities()`` resolves only the first cell of a MINSERT grid + (``multi_insert()`` yields all of them), so a 2x3 grid exploded into one + cell and five vanished with a clean audit. AutoCAD refuses to explode a + MINSERT; so does this.""" + cell = backend._doc.blocks.new(name="CELL") + cell.add_circle((0, 0), 1) + grid = backend._msp().add_blockref( + "CELL", + (0, 0), + dxfattribs={"row_count": 2, "column_count": 3, "row_spacing": 5, "column_spacing": 5}, + ) + assert grid.mcount == 6, "the premise" + before = len(list(backend._msp())) + with pytest.raises(RuntimeError, match=r"MINSERT \(2x3 grid of 'CELL'\)"): + await backend.block_explode(grid.dxf.handle) + assert len(list(backend._msp())) == before + assert backend._doc.entitydb[grid.dxf.handle].is_alive + + +async def test_explode_of_a_single_cell_grid_is_still_an_ordinary_insert(backend): + """A ``row_count``/``column_count`` of 1 is ``mcount`` 1 -- a plain INSERT + wearing MINSERT fields -- and must still explode.""" + cell = backend._doc.blocks.new(name="CELL1") + cell.add_circle((0, 0), 1) + single = backend._msp().add_blockref( + "CELL1", + (0, 0), + dxfattribs={"row_count": 1, "column_count": 1, "row_spacing": 5, "column_spacing": 5}, + ) + assert single.mcount == 1 + result = await backend.block_explode(single.dxf.handle) + assert [(await backend.entity_get(h)).type for h in result["inserted_handles"]] == ["CIRCLE"] + + +async def test_com_explode_turns_a_multi_line_attrib_into_mtext(com): + """``TextString`` of a multi-line attribute is one flattened string; the + content is ``MTextAttributeContent`` and the box ``MTextBoundaryWidth``. + The TEXT path would render ``\\P`` literally, so BURST adds an MTEXT: + height, style and frame first, then the angle, then the attachment the + ATTRIB's alignment names, and the alignment point *last* -- ActiveX's + ``AttachmentPoint`` relocates the anchor (see ``_FakeMText``), and a + BottomRight note anchored before it landed two lines low and a box width + right (live, AutoCAD 2026). A TopLeft attribute is the ``AddMText`` + default and could never show that, so this one is BottomRight.""" + backend, document, space = com + note = _FakeObject( + "AcDbAttribute", + "A1", + TagString="NOTE", + TextString="LINE ONE LINE TWO", + MTextAttribute=True, + MTextAttributeContent="LINE ONE\\PLINE TWO", + MTextBoundaryWidth=40.0, + InsertionPoint=(7.0, 12.7, 0.0), + Alignment=14, # acAlignmentBottomRight: the MTEXT attachment is the alignment point + TextAlignmentPoint=(7.0, 15.2, 0.0), + Height=5.0, + Rotation=0.5236, + StyleName="ISO", + Layer="PID-TAG", + Color=3, + Invisible=False, + ) + plain = _FakeObject( + "AcDbAttribute", + "A2", + TagString="TAG", + TextString="P-101", + MTextAttribute=False, + InsertionPoint=(1.0, 2.0, 0.0), + Height=2.5, + Rotation=0.0, + Layer="0", + Invisible=False, + ) + line = _FakeObject("AcDbLine", "L1") + ref = _FakeObject( + "AcDbBlockReference", + "6D", + Name="TB", + OwnerID=1, + GetAttributes=lambda: (note, plain), + GetConstantAttributes=lambda: (), + Explode=lambda: (line,), + ) + document.objects["6D"] = ref + + result = await backend.block_explode("6D") + + assert result["inserted_handles"] == ["L1"] + assert result["attribute_texts"] == ["T1", "T2"] + assert space.calls == [ + ("AddMText", ((7.0, 15.2, 0.0), 40.0, "LINE ONE\\PLINE TWO")), + ("AddText", ("P-101", (1.0, 2.0, 0.0), 2.5)), + ] + assert space.texts[0].ObjectName == "AcDbMText" + writes = [ + (name, tuple(value.value) if hasattr(value, "value") else value) + for name, value in space.texts[0].writes + ] + assert writes == [ + ("Height", 5.0), + ("StyleName", "ISO"), + ("Rotation", 0.5236), + ("AttachmentPoint", 9), # acAttachmentPointBottomRight + ("InsertionPoint", (7.0, 15.2, 0.0)), # re-asserted after the attachment moved it + ("Layer", "PID-TAG"), + ("Color", 3), + ] + assert tuple(space.texts[0].InsertionPoint.value) == (7.0, 15.2, 0.0), ( + "the MTEXT ends up anchored at the ATTRIB's alignment point" + ) + assert ref.deleted is True + + +async def test_com_explode_multi_line_attrib_takes_the_frame_before_the_angle(com): + backend, document, space = com + note = _FakeObject( + "AcDbAttribute", + "A1", + TagString="NOTE", + TextString="A B", + MTextAttribute=True, + MTextAttributeContent="A\\PB", + InsertionPoint=(-25.0, 13.0, 0.0), + Alignment=0, + Normal=(0.0, 0.0, -1.0), + Height=2.5, + Rotation=3.1416, + Layer="0", + Invisible=True, + ) + ref = _FakeObject( + "AcDbBlockReference", + "7E", + Name="TB", + OwnerID=1, + GetAttributes=lambda: (note,), + GetConstantAttributes=lambda: (), + Explode=lambda: (), + ) + document.objects["7E"] = ref + + result = await backend.block_explode("7E") + + assert result["attribute_texts"] == ["T1"] + assert space.calls == [("AddMText", ((-25.0, 13.0, 0.0), 0.0, "A\\PB"))], ( + "no MTextBoundaryWidth exposed: a zero (unbounded) box" + ) + writes = [ + (name, tuple(value.value) if hasattr(value, "value") else value) + for name, value in space.texts[0].writes + ] + assert writes == [ + ("Height", 2.5), + ("Normal", (0.0, 0.0, -1.0)), + ("Rotation", 3.1416), + ("AttachmentPoint", 7), # acAttachmentPointBottomLeft for a baseline-left ATTRIB + ("InsertionPoint", (-25.0, 13.0, 0.0)), # last: the frame AND the attachment moved it + ("Layer", "0"), + ("Visible", False), + ] + assert tuple(space.texts[0].InsertionPoint.value) == (-25.0, 13.0, 0.0) + + +async def test_com_explode_refuses_an_xref_before_explode(com): + """ActiveX ``Explode()`` raises on an xref; the headless engine used to + delete it. Both name the refusal before anything is dispatched.""" + backend, document, space = com + document.Blocks.blocks["XREF_DETAIL"] = _FakeBlock("XREF_DETAIL", is_xref=True) + dispatched = [] + + def _explode(): + dispatched.append("Explode") + return () + + ref = _FakeObject( + "AcDbBlockReference", + "8A", + Name="XREF_DETAIL", + OwnerID=1, + GetAttributes=lambda: (), + GetConstantAttributes=lambda: (), + Explode=_explode, + ) + document.objects["8A"] = ref + with pytest.raises(RuntimeError, match="external reference 'XREF_DETAIL'"): + await backend.block_explode("8A") + assert dispatched == [] and space.calls == [] and ref.deleted is False + + +async def test_com_explode_refuses_a_minsert_by_name_before_explode(com): + """A MINSERT is ``AcDbMInsertBlock``; it used to fail the INSERT check with + the misleading 'not a block reference'.""" + backend, document, space = com + dispatched = [] + + def _explode(): + dispatched.append("Explode") + return () + + grid = _FakeObject( + "AcDbMInsertBlock", + "9B", + Name="CELL", + OwnerID=1, + Rows=2, + Columns=3, + GetAttributes=lambda: (), + GetConstantAttributes=lambda: (), + Explode=_explode, + ) + document.objects["9B"] = grid + with pytest.raises(RuntimeError, match=r"MINSERT \(2x3 grid of 'CELL'\)"): + await backend.block_explode("9B") + assert dispatched == [] and space.calls == [] and grid.deleted is False + + +# ── Task 2 (review 2): the multi-line writer, and members ezdxf cannot explode ─ + + +async def _insert_multi_line(backend): + """An INSERT of a LINE + ATTDEF block whose ATTRIB is multi-line (embedded + MTEXT ``OLD ONE\\POLD TWO``), built the way the tests above build one.""" + blk = backend._doc.blocks.new(name="MLW") + blk.add_line((0, 0), (10, 0)) + blk.add_attdef("NOTE", insert=(0, 3), text="x", dxfattribs={"height": 2.5}) + ref = await backend.block_insert("MLW", 10, 10, rotation=30.0, attributes={"NOTE": "first"}) + attrib = backend._doc.entitydb[ref.handle].attribs[0] + _embed_multi_line(attrib, ["OLD ONE", "OLD TWO"], attrib.dxf.insert) + return ref, attrib + + +async def test_get_attributes_reports_every_line_of_a_multi_line_attrib(backend): + """``dxf.text`` of an ezdxf-embedded multi-line ATTRIB is only the first + line; AutoCAD's ``TextString`` (what the COM engine reports) is the whole + ``Line1\\PLine2`` content, and so is the headless reader now.""" + ref, _ = await _insert_multi_line(backend) + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "OLD ONE\\POLD TWO"} + + +async def test_set_attributes_rewrites_the_embedded_mtext_of_a_multi_line_attrib(backend): + """The writer used to set only ``dxf.text``, so after an edit the reader + said ``NEW VALUE`` while the explode burst the pre-edit MTEXT -- and an + R2010 save (no embedded MTEXT exported) carried a value the exploded + drawing never showed. One value, on every surface.""" + ref, attrib = await _insert_multi_line(backend) + keys = ("insert", "align_point", "rotation", "height", "halign", "valign", "style") + before = {key: attrib.dxf.get(key) for key in keys} + + result = await backend.block_set_attributes(ref.handle, {"NOTE": "NEW VALUE"}) + + assert result == {"ok": True, "updated_tags": ["NOTE"]} + assert attrib.has_embedded_mtext_entity, "still a multi-line attribute" + assert attrib.virtual_mtext_entity().text == "NEW VALUE" + assert attrib.dxf.text == "NEW VALUE" + after = {key: attrib.dxf.get(key) for key in keys} + assert after == before, "rewriting the content does not move the attribute" + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "NEW VALUE"} + exploded = await backend.block_explode(ref.handle) + (handle,) = exploded["attribute_texts"] + raw = backend._doc.entitydb[handle] + assert raw.dxftype() == "MTEXT" and raw.text == "NEW VALUE" + assert backend._doc.audit().errors == [] + + +async def test_set_attributes_multi_line_value_keeps_every_line(backend): + """A ``\\P`` value on a multi-line attribute is what AutoCAD's + ``TextString`` write takes; every line survives, and the empty value + empties the attribute rather than leaving the old content behind.""" + ref, attrib = await _insert_multi_line(backend) + + await backend.block_set_attributes(ref.handle, {"NOTE": "NEW ONE\\PNEW TWO\\PNEW THREE"}) + + mtext = attrib.virtual_mtext_entity() + assert mtext.plain_text(split=True) == ["NEW ONE", "NEW TWO", "NEW THREE"] + assert attrib.dxf.text == "NEW ONE\\PNEW TWO\\PNEW THREE", ( + "group 1 carries the whole value, AutoCAD's own convention -- ezdxf's " + "first-line mirror is what an R2010 save would keep" + ) + assert await backend.block_get_attributes(ref.handle) == { + "NOTE": "NEW ONE\\PNEW TWO\\PNEW THREE" + } + + await backend.block_set_attributes(ref.handle, {"NOTE": ""}) + assert await backend.block_get_attributes(ref.handle) == {"NOTE": ""} + assert attrib.virtual_mtext_entity().text == "" and attrib.dxf.text == "" + + exploded = await backend.block_explode(ref.handle) + (handle,) = exploded["attribute_texts"] + assert backend._doc.entitydb[handle].text == "" + assert backend._doc.audit().errors == [] + + +async def test_set_attributes_on_a_multi_line_attrib_survives_an_r2010_round_trip( + backend, tmp_path +): + """The default document is R2010, where ezdxf exports no embedded MTEXT: + the saved file carries only ``dxf.text``. ``set_mtext`` mirrors just the + first line there, so a ``\\P`` value once reloaded as ``NEW ONE`` while the + reader had promised three lines. The whole value goes into ``dxf.text`` + (AutoCAD's own convention), so the reloaded drawing keeps every line.""" + ref, _ = await _insert_multi_line(backend) + assert backend._doc.dxfversion == "AC1024", "the premise" + value = "NEW ONE\\PNEW TWO\\PNEW THREE" + await backend.block_set_attributes(ref.handle, {"NOTE": value}) + assert await backend.block_get_attributes(ref.handle) == {"NOTE": value} + path = tmp_path / "ml.dxf" + await backend.drawing_save_as(str(path)) + await backend.drawing_open(str(path)) + + reloaded = next(e for e in backend._msp() if e.dxftype() == "INSERT") + assert not reloaded.attribs[0].has_embedded_mtext_entity, "R2010 drops the embedded MTEXT" + assert await backend.block_get_attributes(reloaded.dxf.handle) == {"NOTE": value}, ( + "the file carries what the reader promised before the save" + ) + exploded = await backend.block_explode(reloaded.dxf.handle) + (handle,) = exploded["attribute_texts"] + raw = backend._doc.entitydb[handle] + assert raw.dxftype() == "TEXT" and raw.dxf.text == value + + +async def test_set_attributes_multi_line_value_survives_an_r2018_round_trip(backend, tmp_path): + """On R2018 the embedded MTEXT is exported too: both surfaces reload with + every line and the reader still prefers the MTEXT.""" + ref, _ = await _insert_multi_line(backend) + backend._doc.dxfversion = "AC1032" + value = "NEW ONE\\PNEW TWO" + await backend.block_set_attributes(ref.handle, {"NOTE": value}) + path = tmp_path / "ml2018.dxf" + await backend.drawing_save_as(str(path)) + await backend.drawing_open(str(path)) + + reloaded = next(e for e in backend._msp() if e.dxftype() == "INSERT") + attrib = reloaded.attribs[0] + assert attrib.has_embedded_mtext_entity, "R2018 keeps the embedded MTEXT" + assert attrib.virtual_mtext_entity().plain_text(split=True) == ["NEW ONE", "NEW TWO"] + assert attrib.dxf.text == value + assert await backend.block_get_attributes(reloaded.dxf.handle) == {"NOTE": value} + assert backend._doc.audit().errors == [] + + +async def test_set_attributes_leaves_a_single_line_attrib_single_line(backend): + _define_tagged_block(backend, "SL") + ref = await backend.block_insert("SL", 0, 0, attributes={"TAG": "P-1"}) + await backend.block_set_attributes(ref.handle, {"TAG": "P-2"}) + attrib = backend._doc.entitydb[ref.handle].attribs[0] + assert not attrib.has_embedded_mtext_entity and attrib.dxf.text == "P-2" + assert await backend.block_get_attributes(ref.handle) == {"TAG": "P-2"} + + +async def test_explode_refuses_members_ezdxf_cannot_explode_before_writing(backend): + """``virtual_entities()`` drops every member it cannot copy or transform + (OLE2FRAME, VIEWPORT) with a debug log, and a proxy entity without proxy + graphics contributes nothing -- so a title block's OLE logo or a vertical + product's proxy geometry used to be destroyed with ``ok: True`` and a + clean audit. Refused by member before anything is written.""" + from ezdxf.entities import factory + + from backends.base import UnsupportedCapabilityError + + blk = backend._doc.blocks.new(name="PRX") + blk.add_line((0, 0), (10, 0)) + for dxftype in ("ACAD_PROXY_ENTITY", "OLE2FRAME", "VIEWPORT"): + blk.add_entity(factory.create_db_entry(dxftype, {}, backend._doc)) + ref = await backend.block_insert("PRX", 0, 0) + before = [e.dxf.handle for e in backend._msp()] + + with pytest.raises(UnsupportedCapabilityError) as excinfo: + await backend.block_explode(ref.handle) + + assert excinfo.value.capability == "explode_opaque_members" + message = str(excinfo.value) + for name in ("ACAD_PROXY_ENTITY", "OLE2FRAME", "VIEWPORT", "'PRX'", "com"): + assert name in message, message + assert [e.dxf.handle for e in backend._msp()] == before, "nothing written" + assert backend._doc.entitydb[ref.handle].is_alive + + +async def test_explode_still_bursts_a_block_every_member_of_which_it_can_explode(backend): + """The refusal is per member, not per block: ordinary geometry keeps + exploding, in definition order, under a uniform scale.""" + blk = backend._doc.blocks.new(name="PLAIN2") + blk.add_line((0, 0), (10, 0)) + blk.add_circle((5, 5), 1) + blk.add_hatch().paths.add_polyline_path([(0, 0), (1, 0), (1, 1)]) + ref = await backend.block_insert("PLAIN2", 0, 0, scale_x=2.0, scale_y=2.0) + result = await backend.block_explode(ref.handle) + types = [(await backend.entity_get(h)).type for h in result["inserted_handles"]] + assert types == ["LINE", "CIRCLE", "HATCH"] + + +async def test_explode_opaque_members_is_declared_on_both_engines(): + from backends.com_backend import ComBackend + from backends.ezdxf_backend import EzdxfBackend + + headless = EzdxfBackend().capabilities().features["explode_opaque_members"] + assert headless.supported is False and "OLE2FRAME" in (headless.reason or "") + assert ComBackend().capabilities().features["explode_opaque_members"].supported is True + + +# ── Task 2 (review 3): find_replace on a multi-line attribute; COM reads propagate ─ + + +async def test_find_replace_rewrites_every_surface_of_a_multi_line_attrib(backend): + """``text_find_replace`` matched and wrote only ``dxf.text`` of an ATTRIB, + so on a multi-line attribute it reported ``replaced: 1, 'OLD ONE' -> + 'NEW ONE'`` while ``block_get_attributes`` still read ``OLD ONE\\POLD TWO`` + and ``block_explode`` burst it. The ATTRIB branch goes through the same + reader and writer the attribute tools use: one value, every surface.""" + ref, attrib = await _insert_multi_line(backend) + + result = await backend.text_find_replace("OLD", "NEW") + + assert result["replaced"] == 1 + (entry,) = result["entities"] + assert entry["type"] == "ATTRIB" + assert entry["before"] == "OLD ONE\\POLD TWO", "the whole value is what was matched" + assert entry["after"] == "NEW ONE\\PNEW TWO", "every line is replaced, not the first" + assert attrib.has_embedded_mtext_entity, "still a multi-line attribute" + assert attrib.virtual_mtext_entity().text == "NEW ONE\\PNEW TWO" + assert attrib.dxf.text == "NEW ONE\\PNEW TWO" + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "NEW ONE\\PNEW TWO"} + exploded = await backend.block_explode(ref.handle) + (handle,) = exploded["attribute_texts"] + raw = backend._doc.entitydb[handle] + assert raw.dxftype() == "MTEXT" and raw.text == "NEW ONE\\PNEW TWO" + assert backend._doc.audit().errors == [] + + +async def test_find_replace_matches_a_line_below_the_first_of_a_multi_line_attrib(backend): + """A needle that only occurs on the second line was invisible to a search + of ``dxf.text`` (the first line alone): ``replaced: 0`` on a value the + reader plainly reports.""" + ref, attrib = await _insert_multi_line(backend) + + result = await backend.text_find_replace("TWO", "2") + + assert result["replaced"] == 1 + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "OLD ONE\\POLD 2"} + assert attrib.virtual_mtext_entity().text == "OLD ONE\\POLD 2" + + +async def test_find_replace_dry_run_reports_the_whole_multi_line_value_and_writes_nothing( + backend, +): + ref, attrib = await _insert_multi_line(backend) + + result = await backend.text_find_replace("OLD", "NEW", dry_run=True) + + assert result["dry_run"] is True and result["replaced"] == 1 + assert result["entities"][0]["before"] == "OLD ONE\\POLD TWO" + assert result["entities"][0]["after"] == "NEW ONE\\PNEW TWO" + assert attrib.virtual_mtext_entity().text == "OLD ONE\\POLD TWO" + assert attrib.dxf.text == "OLD ONE" + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "OLD ONE\\POLD TWO"} + + +async def test_find_replace_rewrites_a_multi_line_attdef_inside_a_block_definition(backend): + """The ATTDEF twin: the default a new INSERT is prompted with lives in the + block definition, and a multi-line one keeps it in an embedded MTEXT.""" + blk = backend._doc.blocks.new(name="MLD") + blk.add_line((0, 0), (10, 0)) + attdef = blk.add_attdef("NOTE", insert=(0, 3), text="x", dxfattribs={"height": 2.5}) + _embed_multi_line(attdef, ["OLD ONE", "OLD TWO"], (0, 3)) + + result = await backend.text_find_replace("OLD", "NEW") + + assert result["replaced"] == 1 and result["entities"][0]["type"] == "ATTDEF" + assert result["entities"][0]["after"] == "NEW ONE\\PNEW TWO" + assert attdef.has_embedded_mtext_entity + assert attdef.virtual_mtext_entity().text == "NEW ONE\\PNEW TWO" + assert attdef.dxf.text == "NEW ONE\\PNEW TWO" + assert backend._doc.audit().errors == [] + + +async def test_find_replace_leaves_a_single_line_attrib_single_line(backend): + _define_tagged_block(backend, "TB3") + ref = await backend.block_insert("TB3", 0, 0, attributes={"TAG": "OLD-1"}) + attrib = backend._doc.entitydb[ref.handle].attribs[0] + + result = await backend.text_find_replace("OLD", "NEW") + + assert result["replaced"] == 1 and result["entities"][0]["after"] == "NEW-1" + assert attrib.dxf.text == "NEW-1" and not attrib.has_embedded_mtext_entity + assert await backend.block_get_attributes(ref.handle) == {"TAG": "NEW-1"} + + +# ── Task 2 (review 4): a moved multi-line attribute keeps one placement ─────── + + +def _placements(attrib) -> tuple[tuple, tuple]: + """(ATTRIB anchor, embedded MTEXT insert), both WCS -- the two surfaces a + multi-line attribute keeps its placement on.""" + anchor = attrib.ocs().to_wcs(attrib.get_placement()[1]) + return tuple(anchor), tuple(attrib.virtual_mtext_entity().dxf.insert) + + +async def _insert_multi_line_plain(backend, name: str = "MLK"): + """The review's repro: unrotated INSERT at (10, 10) of LINE + ATTDEF NOTE, + the ATTRIB made multi-line (``L1\\PL2``) at its own insert (10, 13).""" + blk = backend._doc.blocks.new(name=name) + blk.add_line((0, 0), (10, 0)) + blk.add_attdef("NOTE", insert=(0, 3), text="x", dxfattribs={"height": 2.5}) + ref = await backend.block_insert(name, 10, 10, attributes={"NOTE": "first"}) + attrib = backend._doc.entitydb[ref.handle].attribs[0] + _embed_multi_line(attrib, ["L1", "L2"], attrib.dxf.insert) + assert _placements(attrib) == ((10.0, 13.0, 0.0), (10.0, 13.0, 0.0)), "the premise" + return ref, attrib + + +async def test_move_carries_the_embedded_mtext_of_a_multi_line_attrib(backend): + """``Insert.translate`` moves each ATTRIB's ``insert``/``align_point`` and + nothing else -- ezdxf's ``BaseAttrib`` overrides ``transform`` but not + ``translate`` -- so after ``entity_move`` the embedded MTEXT still sat at + the old place and ``block_explode`` burst it there: the LINE at (60, 60), + the note 50 units away at (10, 13), with a clean audit.""" + ref, attrib = await _insert_multi_line_plain(backend) + + await backend.entity_move(ref.handle, 50, 50) + + assert _placements(attrib) == ((60.0, 63.0, 0.0), (60.0, 63.0, 0.0)) + assert attrib.dxf.text == "L1", "a move does not touch the value" + result = await backend.block_explode(ref.handle) + (line_handle,) = result["inserted_handles"] + (note_handle,) = result["attribute_texts"] + assert tuple(backend._doc.entitydb[line_handle].dxf.start) == (60.0, 60.0, 0.0) + note = backend._doc.entitydb[note_handle] + assert note.dxftype() == "MTEXT" and note.text == "L1\\PL2" + assert tuple(note.dxf.insert) == (60.0, 63.0, 0.0), "the note moved with its symbol" + assert backend._doc.audit().errors == [] + + +async def test_set_attributes_after_a_move_leaves_the_attribute_where_the_move_put_it(backend): + """``_write_attrib_value`` rebuilt the embedded MTEXT from the stale virtual + entity and ``set_mtext`` re-placed the ATTRIB from it: a value edit after a + move teleported the attribute from (60, 63) back to (10, 13) while the + INSERT stayed at (60, 60). Before Task 2 the writer touched only + ``dxf.text`` and the ATTRIB stayed put, so this was a regression.""" + ref, attrib = await _insert_multi_line_plain(backend) + await backend.entity_move(ref.handle, 50, 50) + + await backend.block_set_attributes(ref.handle, {"NOTE": "NEW1\\PNEW2"}) + + assert tuple(backend._doc.entitydb[ref.handle].dxf.insert) == (60.0, 60.0, 0.0) + assert _placements(attrib) == ((60.0, 63.0, 0.0), (60.0, 63.0, 0.0)) + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "NEW1\\PNEW2"} + assert attrib.dxf.text == "NEW1\\PNEW2" + + +async def test_find_replace_after_a_move_leaves_the_attribute_where_the_move_put_it(backend): + ref, attrib = await _insert_multi_line_plain(backend) + await backend.entity_move(ref.handle, 50, 50) + + result = await backend.text_find_replace("L1", "X1") + + assert result["replaced"] == 1 + assert _placements(attrib) == ((60.0, 63.0, 0.0), (60.0, 63.0, 0.0)) + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "X1\\PL2"} + + +async def test_copy_carries_the_embedded_mtext_of_a_multi_line_attrib(backend): + """``entity_copy`` is ``copy()`` + ``translate()``: the copy's ATTRIB moved, + its embedded MTEXT did not, and exploding the copy put the note on the + original.""" + ref, original = await _insert_multi_line_plain(backend) + + copied = await backend.entity_copy(ref.handle, 50, 50) + + attrib = backend._doc.entitydb[copied.handle].attribs[0] + assert _placements(attrib) == ((60.0, 63.0, 0.0), (60.0, 63.0, 0.0)) + assert _placements(original) == ((10.0, 13.0, 0.0), (10.0, 13.0, 0.0)), "the original stays" + result = await backend.block_explode(copied.handle) + (note_handle,) = result["attribute_texts"] + note = backend._doc.entitydb[note_handle] + assert note.text == "L1\\PL2" and tuple(note.dxf.insert) == (60.0, 63.0, 0.0) + assert backend._doc.audit().errors == [] + + +async def test_rectangular_array_carries_the_embedded_mtext_of_a_multi_line_attrib(backend): + ref, _ = await _insert_multi_line_plain(backend) + + cells = await backend.entity_array_rectangular(ref.handle, 1, 3, 0, 20) + + for index, cell in enumerate(cells, start=1): + attrib = backend._doc.entitydb[cell.handle].attribs[0] + expected = (10.0 + 20.0 * index, 13.0, 0.0) + assert _placements(attrib) == (expected, expected), f"cell {index}" + + +async def test_move_of_a_rotated_reference_carries_the_embedded_mtext(backend): + """The rotated fixture the other multi-line tests use: the same delta on + both surfaces, whatever the reference's frame.""" + ref, attrib = await _insert_multi_line(backend) + anchor, mtext = _placements(attrib) + assert anchor == pytest.approx(mtext), "the premise" + + await backend.entity_move(ref.handle, 5, -7, 3) + + moved_anchor, moved_mtext = _placements(attrib) + delta = (5.0, -7.0, 3.0) + assert moved_anchor == pytest.approx(tuple(a + d for a, d in zip(anchor, delta, strict=True))) + assert moved_mtext == pytest.approx(moved_anchor) + + +async def test_explode_bursts_a_multi_line_attrib_at_the_attribs_own_placement(backend): + """A drawing whose two surfaces already disagree (written by a tool that, + like ``Insert.translate``, moved only the ATTRIB): the ATTRIB's own + placement is the attribute's placement -- it is what ``entity_get`` reports + and what a single-line attribute has -- so the burst MTEXT goes there, and + a value write keeps it there instead of snapping to the stale MTEXT.""" + ref, attrib = await _insert_multi_line_plain(backend) + attrib.dxf.insert = (40.0, 41.0, 0.0) + attrib.dxf.align_point = (40.0, 41.0, 0.0) + assert _placements(attrib) == ((40.0, 41.0, 0.0), (10.0, 13.0, 0.0)), "the premise" + + await backend.block_set_attributes(ref.handle, {"NOTE": "N1\\PN2"}) + assert _placements(attrib) == ((40.0, 41.0, 0.0), (40.0, 41.0, 0.0)) + + result = await backend.block_explode(ref.handle) + (note_handle,) = result["attribute_texts"] + assert tuple(backend._doc.entitydb[note_handle].dxf.insert) == (40.0, 41.0, 0.0) + + +async def test_explode_of_a_stale_multi_line_attrib_reads_the_attrib_placement(backend): + """The read-only twin: no write before the explode, the burst still lands + on the ATTRIB's placement.""" + ref, attrib = await _insert_multi_line_plain(backend) + attrib.dxf.insert = (40.0, 41.0, 0.0) + attrib.dxf.align_point = (40.0, 41.0, 0.0) + + result = await backend.block_explode(ref.handle) + + (note_handle,) = result["attribute_texts"] + assert tuple(backend._doc.entitydb[note_handle].dxf.insert) == (40.0, 41.0, 0.0) + + +async def _rotated(backend, handle): + await backend.entity_rotate(handle, 0, 0, 90) + return handle + + +async def _scaled(backend, handle): + await backend.entity_scale(handle, 0, 0, 2.0) + return handle + + +async def _mirrored(backend, handle): + """``entity_mirror`` transforms a *copy* and deletes the original.""" + return (await backend.entity_mirror(handle, 0, 0, 0, 1, delete_original=True)).handle + + +async def _polar_cell(backend, handle): + (cell,) = await backend.entity_array_polar(handle, 2, 180, 0, 0) + await backend.entity_delete(handle) + return cell.handle + + +@pytest.mark.parametrize( + "op", [_rotated, _scaled, _mirrored, _polar_cell], ids=["rotate", "scale", "mirror", "polar"] +) +async def test_transform_keeps_the_whole_multi_line_value_in_group_1(backend, op, tmp_path): + """The ``transform`` twin of the move defect: ezdxf's ``BaseAttrib.transform`` + goes through ``set_mtext``, which mirrors only the first line into + ``dxf.text`` -- so a rotate, scale or mirror after ``block_set_attributes`` + turned ``A\\PB`` back into ``A`` on the surface an R2010 save keeps, and + the reloaded drawing had lost every line but the first.""" + ref, attrib = await _insert_multi_line_plain(backend) + await backend.block_set_attributes(ref.handle, {"NOTE": "A\\PB"}) + assert attrib.dxf.text == "A\\PB", "the premise" + + handle = await op(backend, ref.handle) + + attrib = backend._doc.entitydb[handle].attribs[0] + anchor, mtext = _placements(attrib) + assert anchor == pytest.approx(mtext), "the two surfaces still agree on placement" + assert attrib.dxf.text == "A\\PB" + assert await backend.block_get_attributes(handle) == {"NOTE": "A\\PB"} + path = tmp_path / "ml-transformed.dxf" + await backend.drawing_save_as(str(path)) + await backend.drawing_open(str(path)) + reloaded = next(e for e in backend._msp() if e.dxftype() == "INSERT") + assert await backend.block_get_attributes(reloaded.dxf.handle) == {"NOTE": "A\\PB"} + + +async def test_insert_of_a_multi_line_attdef_keeps_the_whole_value_in_group_1(backend, tmp_path): + """The chain the review names: a title block whose ATTDEF is multi-line + (opened from an AutoCAD file), then ``block_insert``. ezdxf's + ``add_auto_attribs`` embeds the MTEXT through ``embed_mtext``, which + mirrors only the first line into ``dxf.text``, so an R2010 save of a + fresh insert kept ``A`` of ``A\\PB``.""" + blk = backend._doc.blocks.new(name="MLT") + blk.add_line((0, 0), (10, 0)) + attdef = blk.add_attdef("NOTE", insert=(0, 3), text="x", dxfattribs={"height": 2.5}) + _embed_multi_line(attdef, ["D1", "D2"], (0, 3)) + + ref = await backend.block_insert("MLT", 10, 10, attributes={"NOTE": "A\\PB"}) + + attrib = backend._doc.entitydb[ref.handle].attribs[0] + assert attrib.has_embedded_mtext_entity and attrib.dxf.text == "A\\PB" + assert _placements(attrib) == ((10.0, 13.0, 0.0), (10.0, 13.0, 0.0)) + assert await backend.block_get_attributes(ref.handle) == {"NOTE": "A\\PB"} + path = tmp_path / "ml-insert.dxf" + await backend.drawing_save_as(str(path)) + await backend.drawing_open(str(path)) + reloaded = next(e for e in backend._msp() if e.dxftype() == "INSERT") + assert await backend.block_get_attributes(reloaded.dxf.handle) == {"NOTE": "A\\PB"} + + +async def test_move_of_a_single_line_attrib_reference_is_unchanged(backend): + _define_tagged_block(backend, "SLM") + ref = await backend.block_insert("SLM", 0, 0, attributes={"TAG": "P-1"}) + attrib = backend._doc.entitydb[ref.handle].attribs[0] + + await backend.entity_move(ref.handle, 5, 5) + + assert not attrib.has_embedded_mtext_entity + assert tuple(attrib.dxf.insert) == (5.0, 8.0, 0.0) and attrib.dxf.text == "P-1" + + +def _boom(*_args): + raise RuntimeError("RPC_E_CALL_REJECTED: the application is busy") + + +@pytest.mark.parametrize( + "broken", + ["GetAttributes", "GetConstantAttributes", "Invisible"], + ids=["get_attributes", "get_constant_attributes", "invisible"], +) +async def test_com_explode_propagates_a_failing_read_before_explode(com, broken): + """A failing ``GetAttributes()`` used to be swallowed (``attrs = ()``), and + the burst went on to explode, delete every ATTDEF placeholder and the + reference, and return ``ok: True, attribute_texts: []`` -- every value + destroyed silently, the defect the BURST rule exists to remove. A + transient ``RPC_E_CALL_REJECTED`` while AutoCAD is busy is enough to + trigger it. The same for ``GetConstantAttributes()`` (a constant ATTDEF + would then be deleted as a placeholder) and for ``Invisible`` (a hidden + value would appear as visible text). Nothing has been written at that + point, so the failure propagates and the drawing is untouched.""" + backend, document, space = com + dispatched = [] + attdef = _FakeObject("AcDbAttributeDefinition", "D1", Constant=False) + line = _FakeObject("AcDbLine", "L1") + + def _explode(): + dispatched.append("Explode") + return (line, attdef) + + attrib_members = dict( + TagString="TAG", + TextString="P-101", + InsertionPoint=(7.0, 15.2, 0.0), + Height=5.0, + Rotation=0.0, + Layer="PID-TAG", + Invisible=True, + ) + if broken == "Invisible": + attrib_members.pop("Invisible") # the read raises AttributeError on the fake + attrib = _FakeObject("AcDbAttribute", "A1", **attrib_members) + ref_members = dict( + Name="TB", + OwnerID=1, + GetAttributes=lambda: (attrib,), + GetConstantAttributes=lambda: (), + Explode=_explode, + ) + if broken in ref_members: + ref_members[broken] = _boom + ref = _FakeObject("AcDbBlockReference", "2F", **ref_members) + document.objects["2F"] = ref + + with pytest.raises((RuntimeError, AttributeError)): + await backend.block_explode("2F") + + assert dispatched == [], "the read failed before Explode() was dispatched" + assert space.calls == [] and ref.deleted is False and attdef.deleted is False + + +class _SpaceFailingSecondAddText(_FakeSpace): + def AddText(self, text, point, height): + if self.texts: + raise RuntimeError("E_FAIL: AddText") + return super().AddText(text, point, height) + + +async def test_com_explode_undoes_a_burst_that_fails_after_explode(com): + """ActiveX ``Explode()`` leaves the reference in place, so a failure after + it (here ``AddText`` on the second attribute; a constant ATTDEF whose + members cannot be read is the other way in) is put back to that: the + members the explode added and the TEXTs written so far are deleted, the + reference stays, and the failure propagates rather than a half-burst + symbol with ``ok: True``.""" + backend, document, space = com + sheet = _SpaceFailingSecondAddText(name="Sheet") + document.owners[2] = sheet + first = _FakeObject( + "AcDbAttribute", + "A1", + TagString="TAG", + TextString="P-101", + InsertionPoint=(7.0, 15.2, 0.0), + Height=5.0, + Rotation=0.0, + Layer="0", + Invisible=False, + ) + second = _FakeObject( + "AcDbAttribute", + "A2", + TagString="LINK", + TextString="L-1", + InsertionPoint=(1.0, 1.0, 0.0), + Height=2.0, + Rotation=0.0, + Layer="0", + Invisible=False, + ) + placeholder = _FakeObject("AcDbAttributeDefinition", "D1", Constant=False) + line = _FakeObject("AcDbLine", "L1") + ref = _FakeObject( + "AcDbBlockReference", + "4C", + Name="TB", + OwnerID=2, + GetAttributes=lambda: (first, second), + GetConstantAttributes=lambda: (), + Explode=lambda: (line, placeholder), + ) + document.objects["4C"] = ref + + with pytest.raises(RuntimeError, match="E_FAIL: AddText"): + await backend.block_explode("4C") + + assert ref.deleted is False, "the reference Explode() left in place is kept" + assert line.deleted is True, "the member Explode() added is removed" + assert placeholder.deleted is True + assert [t.deleted for t in sheet.texts] == [True], "the TEXT written before the failure" + assert space.calls == [] + + +# ── Task 3: attribute values are validated before any write ────────────────── + + +@pytest.mark.parametrize( + "value, fragment", + [ + (None, "NoneType"), + (True, "bool"), + ({"a": 1}, "dict"), + ([1, 2], "list"), + (float("nan"), "finite"), + ("two\nlines", "single line"), + ], +) +async def test_insert_refuses_a_non_text_attribute_value_by_tag(backend, value, fragment): + _define_tagged_block(backend) + before = len(list(backend._msp())) + with pytest.raises(TypeError, match=r"attributes\['TAG'\]") as exc: + await backend.block_insert("TB", 0, 0, attributes={"TAG": value}) + assert fragment in str(exc.value) + assert len(list(backend._msp())) == before, "a refused insert must not write" + + +async def test_insert_refuses_a_non_mapping_before_writing(backend): + _define_tagged_block(backend) + before = len(list(backend._msp())) + with pytest.raises(TypeError, match="attributes must be an object"): + await backend.block_insert("TB", 0, 0, attributes=[("TAG", "x")]) + assert len(list(backend._msp())) == before + + +async def test_insert_writes_numbers_as_plain_text(backend): + _define_tagged_block(backend) + ref = await backend.block_insert("TB", 0, 0, attributes={"TAG": 101}) + assert await backend.block_get_attributes(ref.handle) == {"TAG": "101"} + ref = await backend.block_insert("TB", 0, 0, attributes={"TAG": 2.5}) + assert await backend.block_get_attributes(ref.handle) == {"TAG": "2.5"} + + +async def test_set_attributes_refuses_by_tag_and_leaves_the_value_alone(backend): + _define_tagged_block(backend) + ref = await backend.block_insert("TB", 0, 0, attributes={"TAG": "P-101"}) + with pytest.raises(TypeError, match=r"attributes\['TAG'\] must be a string or a number"): + await backend.block_set_attributes(ref.handle, {"TAG": None}) + assert await backend.block_get_attributes(ref.handle) == {"TAG": "P-101"} + result = await backend.block_set_attributes(ref.handle, {"TAG": 7}) + assert result == {"ok": True, "updated_tags": ["TAG"]} + assert await backend.block_get_attributes(ref.handle) == {"TAG": "7"} + + +async def test_com_insert_refuses_before_insert_block_and_writes_numbers_as_text(com): + backend, document, space = com + space.attributes = {"TAG": ""} + with pytest.raises(TypeError, match=r"attributes\['TAG'\]"): + await backend.block_insert("TB", 0, 0, attributes={"TAG": None}) + assert space.calls == [], "the refusal fires before InsertBlock" + + await backend.block_insert("TB", 0, 0, attributes={"TAG": 101}) + assert [name for name, _ in space.calls] == ["InsertBlock"] + attr = space.inserted[0].GetAttributes()[0] + assert attr.TextString == "101" and ("TextString", "101") in attr.writes + + +async def test_com_set_attributes_refuses_before_touching_the_reference(com): + backend, document, space = com + attr = _FakeObject("AcDbAttribute", "A1", TagString="TAG", TextString="P-101") + calls = [] + + def get_attributes(): + calls.append("GetAttributes") + return (attr,) + + document.objects["2F"] = _FakeObject( + "AcDbBlockReference", "2F", Name="TB", GetAttributes=get_attributes + ) + with pytest.raises(TypeError, match=r"attributes\['TAG'\]"): + await backend.block_set_attributes("2F", {"TAG": True}) + assert calls == [] and attr.TextString == "P-101" + assert await backend.block_set_attributes("2F", {"TAG": 2.5}) == { + "ok": True, + "updated_tags": ["TAG"], + } + assert attr.TextString == "2.5" + + +# ── Task 4: block_define validates the base point and the layer table ──────── + +CIRC = [{"type": "circle", "cx": 0, "cy": 0, "r": 1}] +ON_LAYER = [{"type": "line", "x1": 0, "y1": 0, "x2": 1, "y2": 1, "layer": "NOT_A_LAYER"}] + + +@pytest.mark.parametrize( + "kwargs, key", + [ + ({"base_x": float("nan")}, "base_x"), + ({"base_y": float("inf")}, "base_y"), + ({"base_x": "3"}, "base_x"), + ({"base_y": True}, "base_y"), + ({"base_x": 10**400}, "base_x"), + ], +) +async def test_define_refuses_a_bad_base_point_before_writing(backend, kwargs, key): + with pytest.raises(TypeError, match=f"'{key}'"): + await backend.block_define("PID_BASE", CIRC, **kwargs) + assert "PID_BASE" not in backend._doc.blocks + + +async def test_define_refuses_a_missing_layer_and_names_it(backend): + with pytest.raises(ValueError, match="'NOT_A_LAYER' do not exist") as exc: + await backend.block_define("PID_LYR", ON_LAYER) + assert "create_layers=true" in str(exc.value) + assert "PID_LYR" not in backend._doc.blocks + assert "NOT_A_LAYER" not in backend._doc.layers + + +async def test_define_creates_the_layer_on_request_and_reports_it(backend): + result = await backend.block_define("PID_LYR", ON_LAYER, create_layers=True) + assert result["layers_created"] == ["NOT_A_LAYER"] and result["ok"] is True + assert "NOT_A_LAYER" in backend._doc.layers + member = next(iter(backend._doc.blocks.get("PID_LYR"))) + assert member.dxf.layer == "NOT_A_LAYER" + + +async def test_define_on_an_existing_layer_reports_nothing_created(backend): + await backend.layer_create("PID-EQUIP") + spec = [{**ON_LAYER[0], "layer": "PID-EQUIP"}] + result = await backend.block_define("PID_OK", spec) + assert result["layers_created"] == [] + result = await backend.block_define("PID_ZERO", CIRC) + assert result["layers_created"] == [], "layer 0 always exists" + + +async def test_com_define_probes_layers_before_blocks_add(com): + backend, document, space = com + with pytest.raises(ValueError, match="'NOT_A_LAYER' do not exist"): + await backend.block_define("PID_LYR", ON_LAYER) + assert "PID_LYR" not in document.Blocks.blocks and document.Layers.added == [] + + result = await backend.block_define("PID_LYR", ON_LAYER, create_layers=True) + assert result["layers_created"] == ["NOT_A_LAYER"] and result["backend"] == "com" + assert document.Layers.added == ["NOT_A_LAYER"] + assert [name for name, _ in document.Blocks.blocks["PID_LYR"].calls] == ["AddLine"] + + +TWO_SPELLINGS = [{**ON_LAYER[0], "layer": "Alpha"}, {**ON_LAYER[0], "layer": "ALPHA"}] + + +async def test_define_names_a_case_folded_missing_layer_once(backend): + """DXF layer names are case-insensitive: 'Alpha' and 'ALPHA' are one missing + layer, refused once, under the first spelling, before anything is written.""" + with pytest.raises(ValueError, match=r"layer\(s\) 'Alpha' do not exist"): + await backend.block_define("PID_CASE", TWO_SPELLINGS) + assert "PID_CASE" not in backend._doc.blocks + assert "Alpha" not in backend._doc.layers + + +async def test_define_creates_a_case_folded_layer_once(backend): + """Without the case fold the second ``layers.add`` raised DXFTableEntryError + after the first one had written — the layer stayed, the block never came.""" + result = await backend.block_define("PID_CASE", TWO_SPELLINGS, create_layers=True) + assert result["layers_created"] == ["Alpha"] and result["entity_count"] == 2 + assert "PID_CASE" in backend._doc.blocks + spellings = [ + layer.dxf.name for layer in backend._doc.layers if layer.dxf.name.lower() == "alpha" + ] + assert spellings == ["Alpha"] + members = [member.dxf.layer for member in backend._doc.blocks.get("PID_CASE")] + assert members == ["Alpha", "ALPHA"], "each primitive keeps the spelling it was given" + + +async def test_define_treats_an_existing_layer_case_insensitively(backend): + await backend.layer_create("PID-EQUIP") + spec = [{**ON_LAYER[0], "layer": "pid-equip"}] + result = await backend.block_define("PID_OK", spec) + assert result["layers_created"] == [] + + +async def test_com_define_creates_a_case_folded_layer_once(com): + backend, document, space = com + with pytest.raises(ValueError, match=r"layer\(s\) 'Alpha' do not exist"): + await backend.block_define("PID_CASE", TWO_SPELLINGS) + assert "PID_CASE" not in document.Blocks.blocks and document.Layers.added == [] + + result = await backend.block_define("PID_CASE", TWO_SPELLINGS, create_layers=True) + assert result["layers_created"] == ["Alpha"] and result["entity_count"] == 2 + assert document.Layers.added == ["Alpha"] + assert [name for name, _ in document.Blocks.blocks["PID_CASE"].calls] == ["AddLine", "AddLine"] + + +async def test_com_define_treats_an_existing_layer_case_insensitively(com): + backend, document, space = com + document.Layers.Add("PID-EQUIP") + spec = [{**ON_LAYER[0], "layer": "pid-equip"}] + result = await backend.block_define("PID_OK", spec) + assert result["layers_created"] == [] and document.Layers.added == ["PID-EQUIP"] + + +async def test_com_define_refuses_a_bad_base_point_before_blocks_add(com): + backend, document, space = com + with pytest.raises(TypeError, match="'base_y'"): + await backend.block_define("PID_BASE", CIRC, base_y=float("nan")) + assert "PID_BASE" not in document.Blocks.blocks + result = await backend.block_define("PID_BASE", CIRC, base_x=1.5, base_y=2.5) + assert document.Blocks.blocks["PID_BASE"].base_point == (1.5, 2.5, 0.0) + assert result["layers_created"] == []