anatoly/autocad-mcp-pro
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(task-10): ensure annotation layers live, and put the weld symbols and their dimensions where ISO 2553 puts them

Browse Source 
Three blocking review findings in engineering/mech/annotate.py.

1. draw_annotation_prims assigned ROLE_LAYER[role] without creating the
   layer. EzdxfBackend._apply_attrs adds a missing layer; ActiveX does not.
   MEASURED live on AutoCAD 2026 (probe: GetActiveObject, Documents.Add(),
   doc.Close(False) in finally): on a scratch document whose layer table is
   ['0'], draw_surface_texture(ComBackend(), at=(0,0), ra=3.2,
   machining='required', lay='perpendicular', allowance=0.5) raised
   "AutoCAD COM error (-0x7ffdfff7)" and left one orphan AcDbLine on layer 0
   — a partial write with no rollback, on exactly the foreign-drawing path
   the spec names (drawing_open runs no bootstrap). The same probe after the
   fix: {'ok': True, count 6, 'layers_created': ['DIM','TEXT']}, model space
   ['AcDbLine' x4 on DIM, 'AcDbText' x2 on TEXT], layer table ['0','DIM',
   'TEXT']. New helper ensure_annotation_layers() creates every target layer
   up front, before the first entity, with its ENGINEERING_LAYERS colour /
   linetype / lineweight; the result reports layers_created.

2. side='both' drew the two elementary symbols 0.4h apart — the other-side
   one still stood on the (correctly omitted) identification line, so a
   double-V butt weld came out as two detached halves that were not even
   symmetric about the reference line. ISO 2553 places them back-to-back on
   the continuous reference line: the other-side baseline is now 0.0 when
   there is no identification line. tests/test_mech_annotate.py asserted the
   wrong apex (-0.4*h), so the gate certified the defect; that test is
   rewritten to assert the mirror pair, plus a double-fillet case.

3. For side='other' the size and length were written above the reference
   line while the symbol hung below the identification line — 'a6' sat where
   an arrow-side throat dimension sits, on a joint with no arrow-side weld.
   Both texts now sit beside their own symbol (0.25h clear of the line it
   stands on, on the symbol's side). New test covers side='other' with size
   and length, and pins arrow/both unchanged.

All five new or rewritten tests fail against the pre-fix module (the layer
one with the measured 'Key not found' refusal, via a fake that models only
that measured behaviour) and pass after. tests/test_mech_annotate.py: 29
passed. Full suite: 3326 passed, 6 pre-existing failures unrelated to this
change (verified identical on the branch tip with the change stashed),
test_release_consistency.py::test_document_numbers_match_the_tree
deselected as instructed. No tool registration changed, so
docs/tool-inventory.json regenerates byte-identical.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
Umut CAN
2026-09-22 19:15:33 +03:00
co-authored by Claude Opus 5
parent a4ce46ba9b
commit c607e75b75
2 changed files with 236 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
engineering/mech/annotate.py
+89 -9
View File
@@ -45,8 +45,10 @@ weld annotation.
from __future__ import annotations
import math
from collections.abc import Iterable
from typing import TYPE_CHECKING
from engineering.layers import ENGINEERING_LAYERS
from engineering.mech.primitives import (
ROLE_LAYER,
Arc,
@@ -73,6 +75,7 @@ __all__ = [
"draw_annotation_prims",
"draw_surface_texture",
"draw_weld_symbol",
"ensure_annotation_layers",
"leader_prims",
"surface_texture_prims",
"symbol_proportions",
@@ -312,8 +315,18 @@ def weld_symbol_prims(
point a leader attaches to. The continuous reference line runs in +X; the
dashed identification line runs parallel below it and is omitted for a
symmetrical weld (`side="both"`), which ISO 2553:2019 permits. The
arrow-side symbol stands on the reference line, the other-side symbol
hangs below the identification line.
arrow-side symbol stands on the reference line and the other-side symbol
hangs below the identification line - except for a symmetrical weld, where
ISO 2553 places the two elementary symbols back-to-back on the continuous
reference line itself (an X butt weld is one symbol, not two detached
halves), so the lower one hangs from y = 0 and the pair is symmetric about
the reference line.
The size and the length/pitch are written beside the symbol they belong
to - size to its left, length to its right - which for an other-side weld
means beside the identification line, not above the reference line: a
throat dimension written on the arrow side of a joint with no arrow-side
weld tells a welder the wrong thing.
"""
if kind not in WELD_KINDS:
raise ValueError(
@@ -360,15 +373,24 @@ def weld_symbol_prims(
prims.append(Line((x, ident_y), (x_end, ident_y), "dim"))
x = x_end + gap
# A symmetrical weld has no identification line, so its other-side symbol
# stands on the reference line; otherwise it stands on the identification
# line. `sense=-1` then grows it downward from that baseline.
other_baseline = 0.0 if side == "both" else ident_y
if side in ("arrow", "both"):
prims.extend(_elementary_prims(kind, x_symbol, 0.0, 1.0, h))
if side in ("other", "both"):
prims.extend(_elementary_prims(kind, x_symbol, ident_y, -1.0, h))
prims.extend(_elementary_prims(kind, x_symbol, other_baseline, -1.0, h))
# The dimensions sit beside their own symbol: 0.25h clear of the line the
# symbol stands on, on the symbol's side of it. For "both" that is the
# arrow-side symbol, which is where ISO 2553 writes a symmetrical weld's
# single set of dimensions.
text_y = ident_y - 1.25 * h if side == "other" else 0.25 * h
if size_text:
prims.append(Text((x_size, 0.25 * h), size_text, h))
prims.append(Text((x_size, text_y), size_text, h))
if right_text:
prims.append(Text((x_right, 0.25 * h), right_text, h))
prims.append(Text((x_right, text_y), right_text, h))
if field_weld:
top = FLAG_POLE_FACTOR * h
@@ -442,6 +464,48 @@ def leader_prims(
)
#: The layer definitions `ROLE_LAYER` names, so a layer this module creates
#: gets the colour/linetype/lineweight `drawing_new` would have given it
#: instead of a bare white Continuous one.
_LAYER_DEFS: dict[str, tuple[int, str, float]] = {
name: (color, linetype, lineweight)
for name, color, linetype, lineweight, _desc in ENGINEERING_LAYERS
}
async def ensure_annotation_layers(
backend: AutoCADBackend, names: Iterable[str]
) -> tuple[str, ...]:
"""Create whichever of `names` the drawing does not have yet; return those.
MEASURED on AutoCAD 2026: assigning `entity.Layer = "DIM"` on a drawing
whose layer table is `['0']` raises `-0x7ffdfff7 ('Key not found')` -
ActiveX will not create the layer for you. The headless engine will
(`EzdxfBackend._apply_attrs` calls `doc.layers.add`), so without this step
the same annotation silently succeeds headless and dies live on the first
entity, having already written the ones before it. `drawing_open` runs no
bootstrap, so annotating a foreign drawing - the case the spec names - is
exactly when the layer is missing. Ensuring every target layer up front,
before any entity is created, is what makes the two engines agree and what
keeps a refusal from leaving half a symbol behind.
"""
wanted: list[str] = []
for name in names:
if name not in wanted:
wanted.append(name)
if not wanted:
return ()
existing = {lyr.name.lower() for lyr in await backend.layer_list()}
created: list[str] = []
for name in wanted:
if name.lower() in existing:
continue
color, linetype, lineweight = _LAYER_DEFS.get(name, (7, "Continuous", 0.25))
await backend.layer_create(name=name, color=color, linetype=linetype, lineweight=lineweight)
created.append(name)
return tuple(created)
async def draw_annotation_prims(
backend: AutoCADBackend,
prims,
@@ -454,7 +518,10 @@ async def draw_annotation_prims(
Rotates about the symbol origin first, then translates to `at`, so every
coordinate that reaches the backend is already WCS. Each primitive lands on
`ROLE_LAYER[role]` unless `layer` overrides it.
`ROLE_LAYER[role]` unless `layer` overrides it, and every target layer is
created first (`ensure_annotation_layers`) - live AutoCAD refuses an
assignment to a layer that is not in the table, and a foreign drawing that
was merely opened has no engineering layers at all.
"""
placed = tuple(prims)
for prim in placed:
@@ -467,9 +534,11 @@ async def draw_annotation_prims(
placed = rotate(placed, float(rotation))
placed = translate(placed, float(at[0]), float(at[1]))
targets = [layer or ROLE_LAYER[prim.role] for prim in placed]
created = await ensure_annotation_layers(backend, targets)
handles: list[str] = []
for prim in placed:
target = layer or ROLE_LAYER[prim.role]
for prim, target in zip(placed, targets, strict=True):
if isinstance(prim, Line):
info = await backend.entity_create_line(
prim.p1[0], prim.p1[1], prim.p2[0], prim.p2[1], 0.0, 0.0, target
@@ -496,7 +565,12 @@ async def draw_annotation_prims(
prim.text, prim.at[0], prim.at[1], prim.height, prim.rotation, target
)
handles.append(info.handle)
return {"ok": True, "handles": handles, "count": len(handles)}
return {
"ok": True,
"handles": handles,
"count": len(handles),
"layers_created": list(created),
}
async def draw_surface_texture(
@@ -538,6 +612,9 @@ async def draw_surface_texture(
drawn = await draw_annotation_prims(backend, leader, layer=layer)
result["handles"].extend(drawn["handles"])
result["count"] = len(result["handles"])
result["layers_created"].extend(
name for name in drawn["layers_created"] if name not in result["layers_created"]
)
result["standard"] = standard
return result
@@ -581,6 +658,9 @@ async def draw_weld_symbol(
drawn = await draw_annotation_prims(backend, leader, layer=layer)
result["handles"].extend(drawn["handles"])
result["count"] = len(result["handles"])
result["layers_created"].extend(
name for name in drawn["layers_created"] if name not in result["layers_created"]
)
result["kind"] = kind
result["side"] = side
return result
tests/test_mech_annotate.py
+147 -4
View File
@@ -8,6 +8,7 @@ document through the `backend` fixture. No screenshots anywhere.
from __future__ import annotations
import math
import types
import pytest
@@ -199,16 +200,38 @@ async def test_the_reference_line_comes_first_and_the_identification_line_is_das
assert dashes[-1].p2[0] <= length + EPS
async def test_a_symmetrical_weld_omits_the_identification_line():
async def test_a_symmetrical_weld_meets_back_to_back_on_the_reference_line():
"""ISO 2553: no identification line, and the two elementary symbols share it.
The earlier version of this test asserted the lower V apex at -0.4h, i.e.
on the (omitted) identification line, which drew a double-V butt weld as
two detached halves that were not even symmetric about the reference line.
"""
prims = weld_symbol_prims(kind="v", side="both", height=3.5)
assert not [
p for p in prims if isinstance(p, Line) and p.p1[1] == pytest.approx(-0.4 * 3.5, abs=EPS)
]
# One V above the reference line, one V below the identification offset.
# One V above the reference line, one below it, both standing on y = 0.
vees = [p for p in prims if isinstance(p, Poly) and len(p.points) == 3 and not p.closed]
assert len(vees) == 2
assert vees[0].points[1][1] == pytest.approx(0.0, abs=EPS)
assert vees[1].points[1][1] == pytest.approx(-0.4 * 3.5, abs=EPS)
upper, lower = vees
assert upper.points[1][1] == pytest.approx(0.0, abs=EPS)
assert lower.points[1][1] == pytest.approx(0.0, abs=EPS)
# Mirror images of each other about y = 0, same x.
assert _flat(lower.points) == pytest.approx(
tuple(v if i % 2 == 0 else -v for i, v in enumerate(_flat(upper.points))), abs=EPS
)
ys = [y for _x, y in upper.points + lower.points]
assert (min(ys), max(ys)) == pytest.approx((-3.5, 3.5), abs=EPS)
async def test_a_symmetrical_fillet_weld_is_the_double_fillet_not_two_halves():
prims = weld_symbol_prims(kind="fillet", side="both", height=3.5)
triangles = [p for p in prims if isinstance(p, Poly) and p.closed]
assert len(triangles) == 2
x0 = triangles[0].points[0][0]
assert _flat(triangles[0].points) == pytest.approx((x0, 0.0, x0, 3.5, x0 + 3.5, 0.0), abs=EPS)
assert _flat(triangles[1].points) == pytest.approx((x0, 0.0, x0, -3.5, x0 + 3.5, 0.0), abs=EPS)
async def test_the_fillet_triangle_stands_on_the_line_with_its_leg_on_the_left():
@@ -228,6 +251,45 @@ async def test_the_other_side_symbol_hangs_below_the_identification_line():
)
async def test_other_side_dimensions_are_written_beside_the_other_side_symbol():
"""ISO 2553 writes the size left of the symbol and the length right of it.
For an other-side weld the symbol hangs from the identification line, so
both texts belong there too. Written above the reference line instead,
`a6` sits exactly where an arrow-side throat dimension sits - on a joint
that has no arrow-side weld.
"""
prims = weld_symbol_prims(kind="fillet", side="other", size=6, length=50, height=3.5)
triangle = [p for p in prims if isinstance(p, Poly) and p.closed][0]
x_symbol = triangle.points[0][0]
texts = [p for p in prims if isinstance(p, Text)]
assert [t.text for t in texts] == ["a6", "50"]
# Beside the symbol: size to its left, length to its right.
assert texts[0].at[0] < x_symbol
assert texts[1].at[0] > x_symbol + 3.5
# Below the reference line, 0.25h clear of the identification line, so the
# whole text body lies between the identification line and the symbol's
# far edge - never above y = 0.
ident_y = -0.4 * 3.5
for t in texts:
assert t.at[1] == pytest.approx(ident_y - 1.25 * 3.5, abs=EPS)
assert t.at[1] + t.height < 0.0
# The arrow side is unchanged: its dimensions stay above the reference line.
arrow = [
p
for p in weld_symbol_prims(kind="fillet", side="arrow", size=6, length=50, height=3.5)
if isinstance(p, Text)
]
assert all(t.at[1] == pytest.approx(0.25 * 3.5, abs=EPS) for t in arrow)
# A symmetrical weld carries one set, beside its arrow-side symbol.
both = [
p
for p in weld_symbol_prims(kind="fillet", side="both", size=6, length=50, height=3.5)
if isinstance(p, Text)
]
assert all(t.at[1] == pytest.approx(0.25 * 3.5, abs=EPS) for t in both)
async def test_the_u_symbol_is_a_half_circle_between_two_uprights():
prims = weld_symbol_prims(kind="u", height=3.5)
arc = [p for p in prims if isinstance(p, Arc)][0]
@@ -304,6 +366,87 @@ from engineering.mech.annotate import ( # noqa: E402 - grouped with its own tes
from engineering.mech.primitives import ROLE_LAYER # noqa: E402
class _StrictLayerBackend:
"""A backend that refuses an entity on a layer the drawing does not have.
MEASURED on AutoCAD 2026: `Documents.Add()` gives a layer table of `['0']`,
and `entity.Layer = "DIM"` on it raises
`AutoCAD COM error (-0x7ffdfff7) ('Key not found')` - live ActiveX never
auto-creates the layer, while `EzdxfBackend._apply_attrs` does. This fake
models that measured refusal (and only it: it has no member the real
`AutoCADBackend` lacks), so the headless suite fails the way the live
engine fails. Before the fix it recorded one LINE and then raised; the
drawing kept that orphan, which is the partial write the fix removes.
"""
def __init__(self, layers=("0",)):
self.layers = {name.lower(): name for name in layers}
self.calls: list[tuple] = []
self._next = 0
async def layer_list(self):
self.calls.append(("layer_list",))
return [types.SimpleNamespace(name=name) for name in self.layers.values()]
async def layer_create(self, name, color=7, linetype="Continuous", lineweight=-3):
self.calls.append(("layer_create", name, color, linetype, lineweight))
self.layers[name.lower()] = name
return types.SimpleNamespace(name=name)
def _entity(self, kind, layer):
if layer is not None and layer.lower() not in self.layers:
raise RuntimeError(f"AutoCAD COM error (-0x7ffdfff7) ('Key not found'): {layer}")
self.calls.append((kind, layer))
self._next += 1
return types.SimpleNamespace(handle=f"{self._next:X}")
async def entity_create_line(self, x1, y1, x2, y2, z1=0.0, z2=0.0, layer=None, **kw):
return self._entity("LINE", layer)
async def entity_create_circle(self, cx, cy, radius, layer=None, **kw):
return self._entity("CIRCLE", layer)
async def entity_create_arc(self, cx, cy, radius, start_angle, end_angle, layer=None, **kw):
return self._entity("ARC", layer)
async def entity_create_polyline(self, points, closed=False, layer=None, **kw):
return self._entity("LWPOLYLINE", layer)
async def entity_create_text(self, text, x, y, height=2.5, rotation=0.0, layer=None, **kw):
return self._entity("TEXT", layer)
async def test_every_target_layer_is_created_before_the_first_entity():
backend = _StrictLayerBackend()
result = await draw_surface_texture(
backend, at=(0.0, 0.0), ra=3.2, machining="required", lay="perpendicular", allowance=0.5
)
assert result["ok"] is True and result["count"] == 6
assert result["layers_created"] == [ROLE_LAYER["dim"], ROLE_LAYER["text"]]
# Nothing is drawn until every layer the symbol needs exists.
first_entity = next(
i
for i, call in enumerate(backend.calls)
if call[0] != "layer_list" and call[0] != "layer_create"
)
assert {call[1] for call in backend.calls[:first_entity] if call[0] == "layer_create"} == {
ROLE_LAYER["dim"],
ROLE_LAYER["text"],
}
# Created with their ENGINEERING_LAYERS definition, not a bare default.
dim_create = next(c for c in backend.calls if c[0] == "layer_create" and c[1] == "DIM")
assert dim_create[2:] == (2, "Continuous", 0.18)
async def test_a_drawing_that_already_has_the_layers_is_not_rewritten():
backend = _StrictLayerBackend(layers=("0", "dim", "TEXT"))
result = await draw_weld_symbol(
backend, at=(0.0, 0.0), kind="fillet", size=5, length=50, leader_to=(-10.0, -10.0)
)
assert result["layers_created"] == []
assert not [c for c in backend.calls if c[0] == "layer_create"]
async def _types_and_layers(backend, handles):
rows = []
for handle in handles: