fix: P0 — COM premium crash, lineweight truncation, ISO critique no-op
Three production-blocking defects + two legal/infra gaps from the recon pass. R1 (CRITICAL): the 8 premium meta-tools (drawing_plan, drawing_critique, point_from_snap, construction_xline, construction_clear, drawing_apply_iso_layers, dimension_auto, entity_select_smart) raised NotImplementedError on the COM backend — i.e. the entire "non-negotiable" premium workflow crashed on the DEFAULT Windows backend. Lifted all eight to AutoCADBackend as concrete, backend-agnostic implementations built on the shared abstract primitives + the identical EntityInfo.properties contract, so both backends inherit them. The only backend-specific piece left is the new `_create_xline` primitive (XLINE has no cross-backend entity_create_* equivalent). Removes ~285 lines of ezdxf-only duplication; the existing meta-tool test suite now covers shared code. R2 (CRITICAL): layer_create/layer_modify/entity_set_properties used int(lineweight), truncating every millimetre lineweight (0.50, 0.25, 0.18, 0.13) to 0 — silently wiping the whole ISO 128 discipline, because engineering/layers.py passes mm-floats while the MCP boundary passes hundredths-ints. Added normalize_lineweight() to reconcile both conventions to ezdxf/COM hundredths and wired it into all six call sites. R3 (HIGH): _check_iso128 was a permanent no-op (everything was 0 from R2, so the `lw <= 0: continue` guard skipped all layers). Fixed by R2; additionally exclude CONSTRUCTION-class layers (intentionally sub-ISO 0.05mm scratch weight) to avoid a new false positive, and corrected the misleading "normalises to mm" comment. Also: add MIT LICENSE (referenced by CLAUDE.md but missing — legal gap) and restore .github/workflows/ci.yml (lint kept non-blocking until the repo-wide ruff cleanup; the test job is the real gate). Verified: 182 passed / 2 xfailed; functional check confirms lineweights now store as hundredths (GEOMETRY=50) and the ISO critique correctly flags a 0.40mm layer while passing a clean ISO drawing.
This commit is contained in:
Umutcan Edizsalan
+337
-21
@@ -86,6 +86,38 @@ class DrawingInfo:
|
||||
backend: str = ""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Shared helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def normalize_lineweight(lw: "float | int | None") -> "int | None":
|
||||
"""Coerce a lineweight to ezdxf/COM hundredths-of-a-mm integers.
|
||||
|
||||
AutoCAD and ezdxf store lineweights as integer hundredths (25 == 0.25 mm),
|
||||
plus the -1/-2/-3 sentinels (ByLayer/ByBlock/Default). Callers, however,
|
||||
mix conventions: the MCP tool boundary passes hundredths (25) while the
|
||||
engineering layer sets pass millimetres (0.25). The old ``int(0.25)``
|
||||
silently truncated millimetre values to 0 and wiped the lineweight (and with
|
||||
it the whole ISO 128 discipline). Disambiguation is unambiguous because no
|
||||
ISO 128 millimetre value exceeds 2.0 and no valid hundredth is below 5, so
|
||||
the ``(0, 2.05]`` band is always millimetres.
|
||||
"""
|
||||
if lw is None:
|
||||
return None
|
||||
try:
|
||||
v = float(lw)
|
||||
except (TypeError, ValueError):
|
||||
return lw # leave exotic / already-int values untouched
|
||||
if v < 0:
|
||||
return int(round(v)) # -1/-2/-3 sentinels
|
||||
if v == 0:
|
||||
return 0
|
||||
if v <= 2.05:
|
||||
return int(round(v * 100.0)) # millimetres -> hundredths
|
||||
return int(round(v)) # already hundredths
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Abstract backend
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -509,7 +541,15 @@ class AutoCADBackend(ABC):
|
||||
async def system_run_lisp(self, expression: str) -> dict: ...
|
||||
|
||||
# ── premium meta-tools (planning + critique + snap + construction) ──────
|
||||
@abstractmethod
|
||||
# These are CONCRETE and backend-agnostic: they orchestrate the abstract
|
||||
# primitives above plus the shared EntityInfo.properties contract (start/
|
||||
# end/center/radius/start_angle/end_angle — populated identically by both
|
||||
# backends), so the ezdxf and COM backends inherit identical premium
|
||||
# behaviour. The only backend-specific primitive is `_create_xline` — XLINE
|
||||
# has no cross-backend equivalent among the standard entity_create_* tools.
|
||||
|
||||
_plan_spec: "PlanSpec | None" = None
|
||||
|
||||
async def drawing_plan(
|
||||
self, intent: str,
|
||||
sheet_size: "SheetSize" = "A3",
|
||||
@@ -520,54 +560,223 @@ class AutoCADBackend(ABC):
|
||||
notes: list[str] | None = None,
|
||||
) -> "PlanSpec":
|
||||
"""Commit a drawing intent before any geometry is created.
|
||||
Returns a PlanSpec replayed at finalize time by `drawing_critique`."""
|
||||
...
|
||||
Returns a PlanSpec held on the backend for `drawing_critique`."""
|
||||
from engineering.plan_spec import PlanSpec
|
||||
plan = PlanSpec(
|
||||
intent=str(intent),
|
||||
sheet_size=sheet_size,
|
||||
scale=float(scale),
|
||||
layer_set_id=layer_set_id,
|
||||
view_count=int(view_count),
|
||||
dim_style=dim_style,
|
||||
notes=list(notes) if notes else [],
|
||||
)
|
||||
self._plan_spec = plan
|
||||
return plan
|
||||
|
||||
@abstractmethod
|
||||
async def drawing_critique(
|
||||
self, focus: list["CritiqueFocus"] | None = None,
|
||||
) -> list["Issue"]:
|
||||
"""Run premium-quality checks; return zero issues before
|
||||
`drawing_finalize`. `focus=None` runs all checks."""
|
||||
...
|
||||
from engineering.critique import run_critique
|
||||
return await run_critique(self, focus)
|
||||
|
||||
@abstractmethod
|
||||
async def point_from_snap(
|
||||
self, handle: str, snap: "SnapType",
|
||||
ref_x: float | None = None, ref_y: float | None = None,
|
||||
) -> tuple[float, float]:
|
||||
"""Return a deterministic snap point on `handle`.
|
||||
snap ∈ {end, mid, center, quad, int, perp, near}.
|
||||
For `near/perp/int`, `ref_x/y` is the reference point or other
|
||||
entity's vicinity."""
|
||||
...
|
||||
snap ∈ {end, mid, center, quad, perp, near}. For `near/perp`,
|
||||
`ref_x/y` is the reference point. Sources geometry from the shared
|
||||
EntityInfo.properties contract — works on every backend."""
|
||||
ent = await self.entity_get(handle)
|
||||
et = ent.type
|
||||
p = ent.properties
|
||||
ref = (
|
||||
(float(ref_x), float(ref_y))
|
||||
if ref_x is not None and ref_y is not None else None
|
||||
)
|
||||
|
||||
def _line_pts():
|
||||
s = p.get("start")
|
||||
e = p.get("end")
|
||||
if s is None or e is None:
|
||||
raise RuntimeError(
|
||||
f"point_from_snap: entity {handle} ({et}) has no start/end."
|
||||
)
|
||||
return (float(s[0]), float(s[1])), (float(e[0]), float(e[1]))
|
||||
|
||||
def _circle_geom():
|
||||
c = p.get("center")
|
||||
r = p.get("radius")
|
||||
if c is None or r is None:
|
||||
raise RuntimeError(
|
||||
f"point_from_snap: entity {handle} ({et}) has no center/radius."
|
||||
)
|
||||
return float(c[0]), float(c[1]), float(r)
|
||||
|
||||
if snap == "end":
|
||||
if et == "LINE":
|
||||
s, e = _line_pts()
|
||||
if ref is None:
|
||||
return s
|
||||
ds = (s[0] - ref[0]) ** 2 + (s[1] - ref[1]) ** 2
|
||||
de = (e[0] - ref[0]) ** 2 + (e[1] - ref[1]) ** 2
|
||||
return s if ds <= de else e
|
||||
if et == "ARC":
|
||||
cx, cy, r = _circle_geom()
|
||||
sa = math.radians(float(p["start_angle"]))
|
||||
ea = math.radians(float(p["end_angle"]))
|
||||
sp = (cx + r * math.cos(sa), cy + r * math.sin(sa))
|
||||
ep = (cx + r * math.cos(ea), cy + r * math.sin(ea))
|
||||
if ref is None:
|
||||
return sp
|
||||
ds = (sp[0] - ref[0]) ** 2 + (sp[1] - ref[1]) ** 2
|
||||
de = (ep[0] - ref[0]) ** 2 + (ep[1] - ref[1]) ** 2
|
||||
return sp if ds <= de else ep
|
||||
raise RuntimeError(f"point_from_snap: 'end' snap not supported on {et}")
|
||||
|
||||
if snap == "mid":
|
||||
if et == "LINE":
|
||||
s, e = _line_pts()
|
||||
return ((s[0] + e[0]) / 2.0, (s[1] + e[1]) / 2.0)
|
||||
if et == "ARC":
|
||||
cx, cy, r = _circle_geom()
|
||||
sa = math.radians(float(p["start_angle"]))
|
||||
ea = math.radians(float(p["end_angle"]))
|
||||
if ea < sa:
|
||||
ea += 2 * math.pi
|
||||
ma = (sa + ea) / 2.0
|
||||
return (cx + r * math.cos(ma), cy + r * math.sin(ma))
|
||||
raise RuntimeError(f"point_from_snap: 'mid' snap not supported on {et}")
|
||||
|
||||
if snap == "center":
|
||||
if et in ("CIRCLE", "ARC", "ELLIPSE"):
|
||||
c = p.get("center")
|
||||
if c is None:
|
||||
raise RuntimeError(
|
||||
f"point_from_snap: entity {handle} ({et}) has no center."
|
||||
)
|
||||
return (float(c[0]), float(c[1]))
|
||||
raise RuntimeError(f"point_from_snap: 'center' snap not supported on {et}")
|
||||
|
||||
if snap == "quad":
|
||||
if et in ("CIRCLE", "ARC"):
|
||||
cx, cy, r = _circle_geom()
|
||||
pts = [(cx + r, cy), (cx, cy + r), (cx - r, cy), (cx, cy - r)]
|
||||
if ref is None:
|
||||
return pts[0]
|
||||
return min(pts, key=lambda q: (q[0] - ref[0]) ** 2 + (q[1] - ref[1]) ** 2)
|
||||
raise RuntimeError(f"point_from_snap: 'quad' snap not supported on {et}")
|
||||
|
||||
if snap == "perp":
|
||||
if ref is None:
|
||||
raise RuntimeError("point_from_snap: 'perp' requires ref_x/ref_y.")
|
||||
if et == "LINE":
|
||||
s, e = _line_pts()
|
||||
dx = e[0] - s[0]
|
||||
dy = e[1] - s[1]
|
||||
L2 = dx * dx + dy * dy
|
||||
if L2 < 1e-18:
|
||||
raise RuntimeError("point_from_snap: line has zero length.")
|
||||
t = ((ref[0] - s[0]) * dx + (ref[1] - s[1]) * dy) / L2
|
||||
return (s[0] + t * dx, s[1] + t * dy)
|
||||
raise RuntimeError(f"point_from_snap: 'perp' snap not supported on {et}")
|
||||
|
||||
if snap == "near":
|
||||
if ref is None:
|
||||
raise RuntimeError("point_from_snap: 'near' requires ref_x/ref_y.")
|
||||
if et == "LINE":
|
||||
s, e = _line_pts()
|
||||
dx = e[0] - s[0]
|
||||
dy = e[1] - s[1]
|
||||
L2 = dx * dx + dy * dy
|
||||
if L2 < 1e-18:
|
||||
return s
|
||||
t = ((ref[0] - s[0]) * dx + (ref[1] - s[1]) * dy) / L2
|
||||
t = max(0.0, min(1.0, t))
|
||||
return (s[0] + t * dx, s[1] + t * dy)
|
||||
if et in ("CIRCLE", "ARC"):
|
||||
cx, cy, r = _circle_geom()
|
||||
vx, vy = ref[0] - cx, ref[1] - cy
|
||||
L = math.hypot(vx, vy)
|
||||
if L < 1e-18:
|
||||
return (cx + r, cy)
|
||||
return (cx + r * vx / L, cy + r * vy / L)
|
||||
raise RuntimeError(f"point_from_snap: 'near' snap not supported on {et}")
|
||||
|
||||
if snap == "int":
|
||||
raise RuntimeError(
|
||||
"point_from_snap: 'int' (intersection) requires a 2nd entity; "
|
||||
"compute via two endpoints. V2 will accept a 2nd handle."
|
||||
)
|
||||
|
||||
raise RuntimeError(
|
||||
f"point_from_snap: unknown snap type '{snap}'. "
|
||||
"Use one of: end, mid, center, quad, perp, near."
|
||||
)
|
||||
|
||||
@abstractmethod
|
||||
async def _create_xline(
|
||||
self, x: float, y: float, dx: float, dy: float, layer: str,
|
||||
) -> EntityInfo:
|
||||
"""Create an infinite construction line (XLINE) through (x, y) with
|
||||
unit direction (dx, dy) on `layer`. Backend-specific: XLINE has no
|
||||
cross-backend entity_create_* equivalent."""
|
||||
...
|
||||
|
||||
async def _ensure_layer(self, name: str, color: int = 7) -> None:
|
||||
"""Idempotently make sure `name` exists (no-op if already present)."""
|
||||
try:
|
||||
existing = {lyr.name for lyr in await self.layer_list()}
|
||||
except Exception:
|
||||
existing = set()
|
||||
if name not in existing:
|
||||
try:
|
||||
await self.layer_create(name, color=color)
|
||||
except Exception:
|
||||
pass # already created / racy create — harmless
|
||||
|
||||
async def construction_xline(
|
||||
self, x: float, y: float, angle_deg: float,
|
||||
layer: str = "CONSTRUCTION",
|
||||
) -> EntityInfo:
|
||||
"""Create an infinite construction line (XLINE) on a CONSTRUCTION
|
||||
layer (auto-created with color 8 dashed if missing)."""
|
||||
...
|
||||
layer (auto-created with color 250 = lightest if missing)."""
|
||||
await self._ensure_layer(layer, color=250)
|
||||
ang = math.radians(float(angle_deg))
|
||||
return await self._create_xline(
|
||||
float(x), float(y), math.cos(ang), math.sin(ang), layer,
|
||||
)
|
||||
|
||||
@abstractmethod
|
||||
async def construction_clear(
|
||||
self, layer: str = "CONSTRUCTION",
|
||||
) -> dict:
|
||||
"""Delete every entity on the CONSTRUCTION layer.
|
||||
Must be called before `drawing_finalize`."""
|
||||
...
|
||||
try:
|
||||
ents = await self.entity_list(layer_filter=layer, limit=5000)
|
||||
except Exception:
|
||||
ents = []
|
||||
deleted = 0
|
||||
for ent in ents:
|
||||
try:
|
||||
await self.entity_delete(ent.handle)
|
||||
deleted += 1
|
||||
except Exception:
|
||||
continue
|
||||
return {"ok": True, "layer": layer, "deleted": deleted}
|
||||
|
||||
@abstractmethod
|
||||
async def drawing_apply_iso_layers(
|
||||
self, standard: "LayerSetId" = "mech",
|
||||
) -> dict:
|
||||
"""Bootstrap a full ISO-conformant layer set with correct colors
|
||||
and lineweights. Idempotent."""
|
||||
...
|
||||
from engineering.layers import apply_layer_set
|
||||
result = await apply_layer_set(self, standard)
|
||||
return {"ok": True, "standard": standard, "layers": result}
|
||||
|
||||
@abstractmethod
|
||||
async def dimension_auto(
|
||||
self, handles: list[str], style: "DimStyle" = "chain",
|
||||
offset: float = 10.0,
|
||||
@@ -575,20 +784,127 @@ class AutoCADBackend(ABC):
|
||||
"""Generate ISO 129 dimensions across `handles` in the chosen style
|
||||
(chain | baseline | ordinate). `offset` is the dimension-line
|
||||
distance from the reference geometry (mm)."""
|
||||
...
|
||||
if not handles:
|
||||
return []
|
||||
if style not in ("chain", "baseline", "ordinate"):
|
||||
raise RuntimeError(
|
||||
f"dimension_auto: unknown style '{style}'. "
|
||||
"Use 'chain', 'baseline', or 'ordinate'."
|
||||
)
|
||||
segs: list[tuple[float, float, float, float]] = []
|
||||
for h in handles:
|
||||
ent = await self.entity_get(h)
|
||||
if ent.type != "LINE":
|
||||
raise RuntimeError(
|
||||
f"dimension_auto V1: only LINE entities supported "
|
||||
f"(handle {h} is {ent.type})."
|
||||
)
|
||||
s = ent.properties.get("start")
|
||||
e = ent.properties.get("end")
|
||||
if s is None or e is None:
|
||||
raise RuntimeError(f"dimension_auto: line {h} missing start/end.")
|
||||
segs.append((float(s[0]), float(s[1]), float(e[0]), float(e[1])))
|
||||
|
||||
off = float(offset)
|
||||
results: list[EntityInfo] = []
|
||||
|
||||
if style == "chain":
|
||||
# Each line gets its own linear (rotated) dimension, offset clear of
|
||||
# the geometry along its own perpendicular.
|
||||
for sx, sy, ex, ey in segs:
|
||||
mx = (sx + ex) / 2.0
|
||||
my = (sy + ey) / 2.0
|
||||
dx, dy = ex - sx, ey - sy
|
||||
L = math.hypot(dx, dy) or 1.0
|
||||
nx, ny = -dy / L, dx / L
|
||||
dim = await self.dimension_linear(
|
||||
sx, sy, ex, ey, mx + nx * off, my + ny * off,
|
||||
rotation=math.degrees(math.atan2(dy, dx)), layer="DIM",
|
||||
)
|
||||
results.append(dim)
|
||||
|
||||
elif style == "baseline":
|
||||
sx0, sy0, ex0, ey0 = segs[0]
|
||||
dx, dy = ex0 - sx0, ey0 - sy0
|
||||
L0 = math.hypot(dx, dy) or 1.0
|
||||
ux, uy = dx / L0, dy / L0
|
||||
for i, (sx, sy, ex, ey) in enumerate(segs):
|
||||
dim = await self.dimension_linear(
|
||||
sx0, sy0, ex, ey,
|
||||
sx0 - uy * (off + i * off), sy0 + ux * (off + i * off),
|
||||
layer="DIM",
|
||||
)
|
||||
results.append(dim)
|
||||
|
||||
else: # ordinate — X then Y distance of each segment's end from start
|
||||
for sx, sy, ex, ey in segs:
|
||||
if abs(ex - sx) > 1e-9:
|
||||
dim = await self.dimension_linear(
|
||||
sx, sy, ex, sy,
|
||||
sx + (ex - sx) / 2.0, max(sy, ey) + off, layer="DIM",
|
||||
)
|
||||
results.append(dim)
|
||||
if abs(ey - sy) > 1e-9:
|
||||
dim = await self.dimension_linear(
|
||||
ex, sy, ex, ey,
|
||||
max(sx, ex) + off, (sy + ey) / 2.0,
|
||||
rotation=90.0, layer="DIM",
|
||||
)
|
||||
results.append(dim)
|
||||
|
||||
return results
|
||||
|
||||
@abstractmethod
|
||||
async def entity_select_smart(
|
||||
self, predicate: dict,
|
||||
) -> list[EntityInfo]:
|
||||
"""Semantic entity selection. Predicate keys (all optional, AND-ed):
|
||||
- type: "LINE" | "CIRCLE" | ...
|
||||
- layer: layer name
|
||||
- near: [x, y, radius] — entity bounding box must intersect circle
|
||||
- near: [x, y, radius] — entity reference point within circle
|
||||
- length_range: [min, max] — applies to LINE/ARC
|
||||
- color: ACI int
|
||||
"""
|
||||
...
|
||||
if not isinstance(predicate, dict):
|
||||
raise RuntimeError("entity_select_smart: predicate must be a dict.")
|
||||
ptype = predicate.get("type")
|
||||
player = predicate.get("layer")
|
||||
pnear = predicate.get("near") # [x, y, radius]
|
||||
plen = predicate.get("length_range") # [min, max]
|
||||
pcolor = predicate.get("color")
|
||||
|
||||
ents = await self.entity_list(
|
||||
type_filter=ptype.upper() if isinstance(ptype, str) else None,
|
||||
layer_filter=player if isinstance(player, str) else None,
|
||||
limit=5000,
|
||||
)
|
||||
|
||||
def _ok(e: EntityInfo) -> bool:
|
||||
if pcolor is not None and int(e.color) != int(pcolor):
|
||||
return False
|
||||
if pnear is not None:
|
||||
try:
|
||||
nx, ny, nr = float(pnear[0]), float(pnear[1]), float(pnear[2])
|
||||
except Exception:
|
||||
return False
|
||||
pt = (e.properties.get("start")
|
||||
or e.properties.get("center")
|
||||
or e.properties.get("insertion")
|
||||
or e.properties.get("insertion_point"))
|
||||
if pt is None:
|
||||
return False
|
||||
if (float(pt[0]) - nx) ** 2 + (float(pt[1]) - ny) ** 2 > nr ** 2:
|
||||
return False
|
||||
if plen is not None and e.type in ("LINE", "ARC"):
|
||||
try:
|
||||
lmin, lmax = float(plen[0]), float(plen[1])
|
||||
except Exception:
|
||||
return False
|
||||
length = e.properties.get("length")
|
||||
if length is None or not (lmin <= float(length) <= lmax):
|
||||
return False
|
||||
return True
|
||||
|
||||
return [e for e in ents if _ok(e)]
|
||||
|
||||
# ── concrete helpers (use existing primitives) ──────────────────────────
|
||||
async def set_layer_active(self, name: str) -> None:
|
||||
|
||||
Reference in New Issue
Block a user