From 375c9f4c70cbac7f1a5e6bd1613e566aa2b00f1f Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Fri, 18 Sep 2026 00:11:38 +0300 Subject: [PATCH] fix(blocks): block_define refuses a non-finite base point and a missing layer unless create_layers base_x/base_y were only float()-cast, so a NaN base point was written into the BLOCK record; a primitive on a layer the drawing lacks was written as a dangling reference doc.audit() never flags (and on COM would fail mid-loop after Blocks.Add). validate_base_point names the key; the layer table is probed before the first write and the missing names are refused, or created and reported as layers_created when create_layers=true (new optional kwarg, contract + both engines + tool). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- backends/block_specs.py | 17 +++++++- backends/com_backend.py | 30 ++++++++++++-- backends/contracts/blocks.py | 14 ++++++- backends/ezdxf_backend.py | 28 ++++++++++--- server.py | 23 ++++++++--- tests/test_block_define.py | 1 + tests/test_track_a_hardening.py | 69 +++++++++++++++++++++++++++++++++ 7 files changed, 165 insertions(+), 17 deletions(-) diff --git a/backends/block_specs.py b/backends/block_specs.py index 5471331..587f3bc 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,18 @@ 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 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. diff --git a/backends/com_backend.py b/backends/com_backend.py index 1b3d8ca..7fe5f69 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -3880,18 +3880,24 @@ 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 ( solid_vertices, validate_attdef_specs, + validate_base_point, validate_entity_specs, ) @@ -3903,6 +3909,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 = sorted({spec["layer"] for spec in ents if spec["layer"] != "0"}) # acAlignmentLeft / Center / Right / MiddleCenter align_map = {"left": 0, "center": 1, "right": 2, "middle_center": 10} @@ -3923,7 +3931,22 @@ 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 = [] + 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: @@ -3989,6 +4012,7 @@ class ComBackend(AutoCADBackend): "entity_count": len(ents), "attdef_count": len(atts), "replaced": replaced, + "layers_created": missing_layers, "backend": "com", } 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 8097923..e5c8fed 100644 --- a/backends/ezdxf_backend.py +++ b/backends/ezdxf_backend.py @@ -4976,20 +4976,26 @@ 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 ( solid_vertices, validate_attdef_specs, + validate_base_point, validate_entity_specs, ) from security import sanitize_symbol_name @@ -5002,6 +5008,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 = sorted({spec["layer"] for spec in ents if spec["layer"] != "0"}) align_map = { "left": TextEntityAlignment.LEFT, "center": TextEntityAlignment.CENTER, @@ -5018,7 +5026,16 @@ class EzdxfBackend(AutoCADBackend): "pass overwrite=true to replace its contents" ) replaced = existing is not None - base = (float(base_x), float(base_y), 0.0) + 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: @@ -5085,6 +5102,7 @@ class EzdxfBackend(AutoCADBackend): "entity_count": len(ents), "attdef_count": len(atts), "replaced": replaced, + "layers_created": missing_layers, "backend": "ezdxf", } diff --git a/server.py b/server.py index 032f2e3..b0fe247 100644 --- a/server.py +++ b/server.py @@ -3160,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") 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_track_a_hardening.py b/tests/test_track_a_hardening.py index bfbfd76..a6bf89c 100644 --- a/tests/test_track_a_hardening.py +++ b/tests/test_track_a_hardening.py @@ -1589,3 +1589,72 @@ async def test_com_set_attributes_refuses_before_touching_the_reference(com): "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"] + + +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"] == []