From 5388440c274c5e5240b30bc142e4f8297d12e7b4 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Wed, 23 Sep 2026 12:20:25 +0300 Subject: [PATCH] fix(task-7): rotation honoured end to end, each view dimensioned once, refusals that name their path - rotation turns the whole sheet of views about `at` as one rigid body: the prims, the recorded WCS `bbox` (plus the sheet-frame `frame_bbox` the next view is laid out against), the detail marker, and every dimension intent (points, text position, and the linear measuring direction + rotation). Measured headless: rotation=90 at (100,100) now draws (70,100)-(100,172), records exactly that box, and leaves only the 2 diameter chord defpoints off-geometry (was 4, up to 47.5 mm away). - dimension_part dimensions only views whose record is not `dimensioned`, takes `views=[indices]`, refuses a view already dimensioned by name (ISO 129-1), writes the flag back, places the hole table once, and refuses a fit no target view claims before drawing anything. A view the dimension layer emits nothing for (a detail) is reported in `skipped` and left unflagged. mech_dimension_part gains the `views` argument. - draw_prims checks hatch_add_boundary's {"ok": False}: the hatch is removed and the failure raised, never left solid through the bore. An engine whose capability map says `hatch_edge_paths` is unsupported (the live one) is refused before any prim of the view is drawn. - a view scale other than 1.0 is refused by name in draw_part and add_view (views are drawn full size; the scale would only have been recorded). - add_view's collision step compares real rectangles on both axes and steps along the projection axis layout_origin chose, so a third-angle view is never shoved back onto the front view and a top view keeps its alignment. - draw_from_spec validates `at` / `rotation` into the `parts[i]:` path and a transaction that cannot open is a ValueError (a ToolError at the tool). No new ActiveX member is called: the change composes existing contract members (dimension_linear rotation, entity_delete, capabilities()). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/mech/draw.py | 421 +++++++++++++++++++++++++++++++++------ server.py | 46 ++++- tests/test_mech_draw.py | 211 ++++++++++++++++++++ 3 files changed, 613 insertions(+), 65 deletions(-) diff --git a/engineering/mech/draw.py b/engineering/mech/draw.py index cb4a598..0561412 100644 --- a/engineering/mech/draw.py +++ b/engineering/mech/draw.py @@ -106,8 +106,9 @@ async def draw_prims( rotation: float = 0.0, layer_prefix: str = "", ) -> dict: - """Draw primitive descriptions, placed at ``at`` and turned by ``rotation``.""" + """Draw primitive descriptions, turned by ``rotation`` about their origin, then placed at ``at``.""" placed = tuple(prims) + _refuse_islands_without_edge_paths(backend, placed) if abs(float(rotation)) > 1e-12: placed = rotate(placed, float(rotation), (0.0, 0.0)) if abs(float(at[0])) > 1e-12 or abs(float(at[1])) > 1e-12: @@ -157,7 +158,7 @@ async def draw_prims( style["angle"], layer=layer, ) - for island in prim.loops[1:]: + for number, island in enumerate(prim.loops[1:], start=1): points = list(island) edges = [ { @@ -167,7 +168,24 @@ async def draw_prims( } for i in range(len(points)) ] - await backend.hatch_add_boundary(info.handle, edges) + # `hatch_add_boundary` answers a bad edge list or a bad handle + # with {"ok": False} rather than raising. An island that did not + # attach leaves the hatch solid straight through the bore it was + # meant to exclude, so the hatch is taken back out and the + # failure is raised - a missing hatch is honest, a wrong one is not. + try: + added = await backend.hatch_add_boundary(info.handle, edges) + except Exception: + await _discard(backend, info.handle) + raise + if not isinstance(added, dict) or not added.get("ok"): + await _discard(backend, info.handle) + error = added.get("error") if isinstance(added, dict) else added + raise ValueError( + f"hatch island {number} of {len(prim.loops) - 1} ({prim.material}) " + f"did not attach: {error}; the hatch was removed rather than left " + "filling the material it was meant to exclude." + ) else: # pragma: no cover - the Prim union is closed raise ValueError(f"draw_prims: unknown primitive {type(prim).__name__}.") handles.append(info.handle) @@ -177,11 +195,111 @@ async def draw_prims( return {"handles": handles, "counts": counts} +def _refuse_islands_without_edge_paths(backend, prims) -> None: + """Refuse a hatch with islands on an engine that cannot attach them - before drawing. + + Islands go through `hatch_add_boundary`, which the live engine refuses + (capability ``hatch_edge_paths``: ActiveX appends loops as objects, not + typed edges). Finding that out at the hatch would leave the rest of the + view already drawn; asking the capability map first leaves nothing behind. + """ + if not any(isinstance(p, HatchArea) and len(p.loops) > 1 for p in prims): + return + try: + feature = backend.capabilities().features.get("hatch_edge_paths") + except Exception: # a backend without a capability map: let the member answer + return + if feature is not None and not feature.supported: + from backends.capability import UnsupportedCapabilityError + + raise UnsupportedCapabilityError( + "hatch_edge_paths", + "this view hatches a cut face with an island (a bore or a hole through the " + f"section), and the {getattr(backend, 'name', 'current')} backend cannot attach " + f"hatch islands ({feature.reason}); nothing was drawn. Draw this view on the " + "headless backend (AUTOCAD_MCP_BACKEND=ezdxf).", + ) + + +async def _discard(backend, handle: str) -> None: + """Delete an entity this module just made; a failure here must not mask the cause.""" + try: + await backend.entity_delete(handle) + except Exception as exc: # pragma: no cover - the original error is the one to raise + log.warning("draw_prims: could not remove hatch %s after a failed island: %s", handle, exc) + + +# -- the placement: the sheet frame and WCS ---------------------------------- +# +# Every view of a part is laid out in one *sheet frame*: the unrotated frame in +# which the first view's lower-left corner sits at `placement.at`. Rotation is +# then applied to the whole sheet as a rigid body about `at`, so the views stay +# on their projection axes and the anchor POINT stays on the part's corner. +# A view record keeps both boxes: `frame_bbox` is what `add_view` lays the next +# view out against, `bbox` is the WCS extent of what was drawn. + + +def _turn(point, at, rotation: float) -> tuple[float, float]: + x, y = float(point[0]), float(point[1]) + if abs(rotation) <= 1e-12: + return (x, y) + cos = math.cos(math.radians(rotation)) + sin = math.sin(math.radians(rotation)) + dx, dy = x - float(at[0]), y - float(at[1]) + return (float(at[0]) + dx * cos - dy * sin, float(at[1]) + dx * sin + dy * cos) + + +def _to_wcs(prims, offset, at, rotation: float) -> tuple: + """Primitives in a view's own coordinates -> WCS: offset into the frame, turn about ``at``.""" + placed = translate(prims, float(offset[0]), float(offset[1])) + if abs(rotation) > 1e-12: + placed = rotate(placed, float(rotation), (float(at[0]), float(at[1]))) + return placed + + +def _wcs_bbox(frame_bbox, at, rotation: float) -> list[float]: + x0, y0, x1, y1 = (float(v) for v in frame_bbox) + corners = [_turn(p, at, rotation) for p in ((x0, y0), (x1, y0), (x1, y1), (x0, y1))] + xs = [p[0] for p in corners] + ys = [p[1] for p in corners] + return [min(xs), min(ys), max(xs), max(ys)] + + +def _frame_bbox(record: dict) -> list[float]: + """The sheet-frame box of a record (a payload written before it existed: its WCS box).""" + return [float(v) for v in (record.get("frame_bbox") or record["bbox"])] + + +def _full_size(scale, where: str) -> float: + """Refuse a view scale the geometry would not have. + + The view engine builds every view but a detail at 1:1, so a scale here + would only be *recorded* - and `mech_part_inspect` would then report a + scale the drawing does not have. Refused by name until it is honoured. + """ + value = _number(scale, where) + if abs(value - 1.0) > 1e-12: + raise ValueError( + f"{where}: {value:g} is refused - views are drawn full size (1:1) in this build, " + "so the scale would be recorded but not drawn. Scale the sheet through a paper-space " + "viewport (viewport_set_scale); a detail view takes its own scale in detail['scale']." + ) + return value + + # -- dimensions -------------------------------------------------------------- -async def _draw_dimension(backend, intent) -> tuple[str | None, dict]: - """One laid-out intent as a real DIMENSION. Returns (handle, the resolved row).""" +async def _draw_dimension( + backend, intent, *, linear_angle: float | None = None +) -> tuple[str | None, dict]: + """One laid-out intent as a real DIMENSION. Returns (handle, the resolved row). + + ``linear_angle`` is the measuring direction of a linear intent when the + caller knows it better than the points do: a part turned by its placement + rotation measures along its own axes, which the WCS points alone no longer + reveal. ``None`` reads it from the points (horizontal or vertical). + """ resolved = resolve_tolerance(intent) row = { "feature": intent.feature, @@ -202,7 +320,9 @@ async def _draw_dimension(backend, intent) -> tuple[str | None, dict]: } if intent.kind == "linear": - horizontal = abs(intent.p2[1] - intent.p1[1]) <= abs(intent.p2[0] - intent.p1[0]) + if linear_angle is None: + horizontal = abs(intent.p2[1] - intent.p1[1]) <= abs(intent.p2[0] - intent.p1[0]) + linear_angle = 0.0 if horizontal else 90.0 info = await backend.dimension_linear( intent.p1[0], intent.p1[1], @@ -210,7 +330,7 @@ async def _draw_dimension(backend, intent) -> tuple[str | None, dict]: intent.p2[1], text_at[0], text_at[1], - 0.0 if horizontal else 90.0, + float(linear_angle), ROLE_LAYER["dim"], **tol, ) @@ -275,14 +395,26 @@ async def _draw_dimension(backend, intent) -> tuple[str | None, dict]: # -- the part payload -------------------------------------------------------- -def _view_record(view: View, offset: tuple[float, float], kw: dict) -> dict: +def _view_record( + view: View, + offset: tuple[float, float], + kw: dict, + at: tuple[float, float] = (0.0, 0.0), + rotation: float = 0.0, +) -> dict: x0, y0, x1, y1 = view.bbox + frame = [x0 + offset[0], y0 + offset[1], x1 + offset[0], y1 + offset[1]] return { "kind": view.kind, "label": view.label, "scale": view.scale, "offset": [float(offset[0]), float(offset[1])], - "bbox": [x0 + offset[0], y0 + offset[1], x1 + offset[0], y1 + offset[1]], + # `bbox` is the WCS extent of what was drawn - the part turned by the + # placement rotation - and `frame_bbox` the same box in the sheet frame + # the next view is laid out in. They are equal while rotation is 0. + "bbox": _wcs_bbox(frame, at, rotation), + "frame_bbox": frame, + "dimensioned": False, "kw": kw, } @@ -319,6 +451,12 @@ def _view_from_record(part, payload: dict, record: dict) -> View: ) +def _placement(payload: dict) -> tuple[tuple[float, float], float]: + placement = payload.get("placement") or {} + at = placement.get("at") or [0.0, 0.0] + return (float(at[0]), float(at[1])), float(placement.get("rotation", 0.0)) + + async def draw_part( backend, part, @@ -331,8 +469,22 @@ async def draw_part( style: str = "chain", scale: float = 1.0, ) -> dict: - """Draw a part's views, dimension them, and write the model onto the drawing.""" + """Draw a part's views, dimension them, and write the model onto the drawing. + + ``at`` is where the first view's lower-left corner goes, and ``rotation`` + turns the whole sheet of views about that point as one rigid body, so the + views keep their projection axes and the dimensions turn with them. + ``scale`` must be 1.0: every view is drawn full size in this build and a + recorded-but-undrawn scale is refused rather than written into the model. + """ validate_part(part) + try: + at_x, at_y = at + except (TypeError, ValueError): + raise ValueError(f"at: expected [x, y], got {at!r}.") from None + origin = (_number(at_x, "at[0]", positive=False), _number(at_y, "at[1]", positive=False)) + turn = _number(rotation, "rotation", positive=False) + scale = _full_size(scale, "scale") kinds = [str(kind).strip().lower() for kind in (views or ())] if not kinds: raise ValueError("views: name at least one view to draw.") @@ -346,7 +498,7 @@ async def draw_part( for index, kind in enumerate(kinds): view = build_view(part, kind, projection=projection, scale=scale) if index == 0: - offset = _offset_for(view, (float(at[0]), float(at[1]))) + offset = _offset_for(view, origin) else: lower_left = layout_origin( _placed(parent, parent_offset), @@ -356,14 +508,14 @@ async def draw_part( child=view, ) offset = _offset_for(view, lower_left) - drawn = await draw_prims(backend, view.prims, at=offset, rotation=rotation) + drawn = await draw_prims(backend, _to_wcs(view.prims, offset, origin, turn)) handles.extend(drawn["handles"]) omitted.extend(view.omitted) - records.append(_view_record(view, offset, {"scale": scale})) + records.append(_view_record(view, offset, {"scale": scale}, origin, turn)) if index == 0: parent, parent_offset = view, offset - anchor = await backend.entity_create_point(float(at[0]), float(at[1]), layer=ANCHOR_LAYER) + anchor = await backend.entity_create_point(origin[0], origin[1], layer=ANCHOR_LAYER) payload = { "v": PAYLOAD_VERSION, "kind": "part", @@ -371,8 +523,8 @@ async def draw_part( "view": records[0]["kind"], "part": part_to_dict(part), "placement": { - "at": [float(at[0]), float(at[1])], - "rotation": float(rotation), + "at": [origin[0], origin[1]], + "rotation": turn, "projection": projection, }, "views": records, @@ -388,6 +540,8 @@ async def draw_part( } if dimension: dimensioned = await dimension_part(backend, anchor.handle, style=style) + for index in dimensioned["views_dimensioned"]: + records[index]["dimensioned"] = True result["dimensions"] = dimensioned["count"] result["handles"].extend(dimensioned["handles"]) return result @@ -424,6 +578,45 @@ async def read_part(backend, part_id: str | None = None) -> dict: return {"part_id": handle, **payload} +def _overlaps(box, other, gap: float) -> bool: + """Do two frame rectangles come closer than ``gap`` on *both* axes?""" + eps = 1e-6 + return ( + box[0] < other[2] + gap - eps + and box[2] > other[0] - gap + eps + and box[1] < other[3] + gap - eps + and box[3] > other[1] - gap + eps + ) + + +def _clear_of(records, lower_left, width: float, height: float, parent_box, gap: float): + """Step a new view past every view already placed, away from its parent. + + `layout_origin` put the view on its projection axis (left, right, above or + below the parent); a collision is resolved by stepping further along that + same axis, so a third-angle end view is never shoved back into the front + view it was placed away from, and a view that only shares an X range with + another (a top view under the front) is not a collision at all. + """ + cx = lower_left[0] + width / 2.0 + cy = lower_left[1] + height / 2.0 + dx = cx - (parent_box[0] + parent_box[2]) / 2.0 + dy = cy - (parent_box[1] + parent_box[3]) / 2.0 + boxes = [_frame_bbox(record) for record in records] + for _ in range(len(boxes) + 1): + box = (lower_left[0], lower_left[1], lower_left[0] + width, lower_left[1] + height) + hit = next((other for other in boxes if _overlaps(box, other, gap)), None) + if hit is None: + return lower_left + if abs(dx) >= abs(dy): + x = hit[2] + gap if dx > 0 else hit[0] - gap - width + lower_left = (x, lower_left[1]) + else: + y = hit[3] + gap if dy > 0 else hit[1] - gap - height + lower_left = (lower_left[0], y) + return lower_left # pragma: no cover - each step clears one box for good + + async def add_view(backend, part_id: str, kind: str, **kw) -> dict: """Add another view of a part already on the drawing, laid out against its first.""" payload = await read_part(backend, part_id) @@ -431,9 +624,8 @@ async def add_view(backend, part_id: str, kind: str, **kw) -> dict: records = list(payload.get("views") or []) if not records: raise ValueError(f"{part_id}: the payload carries no view to lay this one out against.") - placement = payload.get("placement") or {} - projection = str(placement.get("projection", "first")) - rotation = float(placement.get("rotation", 0.0)) + projection = str((payload.get("placement") or {}).get("projection", "first")) + origin, rotation = _placement(payload) plane = kw.pop("plane", None) if plane is not None: @@ -442,7 +634,7 @@ async def add_view(backend, part_id: str, kind: str, **kw) -> dict: gap = float(kw.pop("gap", DEFAULT_GAP)) side = str(kw.pop("side", "right")) style = str(kw.pop("style", "full")) - scale = float(kw.pop("scale", 1.0)) + scale = _full_size(kw.pop("scale", 1.0), "scale") if kw: raise ValueError(f"add_view: unknown argument(s) {sorted(kw)}.") @@ -459,29 +651,31 @@ async def add_view(backend, part_id: str, kind: str, **kw) -> dict: parent_record = records[0] parent_view = _view_from_record(part, payload, parent_record) + parent_frame = _placed(parent_view, parent_record["offset"]) lower_left = layout_origin( - _placed(parent_view, parent_record["offset"]), + parent_frame, kind, side=side, projection=projection, gap=gap, child=view, ) - # step past every view already placed, so a third view never lands on the second - width = view.bbox[2] - view.bbox[0] - for record in records[1:]: - x0, y0, x1, y1 = record["bbox"] - del y0, y1 - if lower_left[0] < x1 + gap and lower_left[0] + width > x0 - gap: - lower_left = (x0 - gap - width, lower_left[1]) + lower_left = _clear_of( + records, + lower_left, + view.bbox[2] - view.bbox[0], + view.bbox[3] - view.bbox[1], + parent_frame.bbox, + gap, + ) offset = _offset_for(view, lower_left) - drawn = await draw_prims(backend, view.prims, at=offset, rotation=rotation) + drawn = await draw_prims(backend, _to_wcs(view.prims, offset, origin, rotation)) marker_handles: list[str] = [] if kind == "detail": marker = detail_marker_prims(detail or {}) marker_drawn = await draw_prims( - backend, marker, at=parent_record["offset"], rotation=rotation + backend, _to_wcs(marker, parent_record["offset"], origin, rotation) ) marker_handles = marker_drawn["handles"] @@ -496,6 +690,8 @@ async def add_view(backend, part_id: str, kind: str, **kw) -> dict: "plane": _plane_to_json(plane), "detail": detail, }, + origin, + rotation, ) ) payload["views"] = records @@ -542,6 +738,47 @@ def _fits_for_view(part, view, fits: dict, *, style: str, threshold: int) -> dic return {key: code for key, code in fits.items() if str(key) in available} +def _view_name(index: int, record: dict) -> str: + return f"{index} ({record.get('label') or record.get('kind')})" + + +def _dimension_targets(part_id: str, records: list[dict], views) -> list[int]: + """Which views this call dimensions - never one that already carries its dimensions. + + ISO 129-1: each dimension appears once. A view record carries a + ``dimensioned`` flag, so a second call (or `mech_dimension_part` after a + `mech_part_draw` that already dimensioned) adds only the views that have + none, and naming a dimensioned view is refused rather than stacked. + """ + if views is None: + targets = [i for i, record in enumerate(records) if not record.get("dimensioned")] + if not targets: + names = ", ".join(_view_name(i, r) for i, r in enumerate(records)) + raise ValueError( + f"{part_id}: every view is already dimensioned ({names}); each measurement " + "appears once (ISO 129-1). Add a view with mech_view_add, then dimension it." + ) + return targets + if isinstance(views, (str, bytes)) or not isinstance(views, (list, tuple)) or not views: + raise ValueError(f"views: expected a list of view indices such as [1], got {views!r}.") + targets: list[int] = [] + for n, value in enumerate(views): + if isinstance(value, bool) or not isinstance(value, int): + raise ValueError(f"views[{n}]: expected a view index (int), got {value!r}.") + if not 0 <= value < len(records): + names = ", ".join(_view_name(i, r) for i, r in enumerate(records)) + raise ValueError(f"views[{n}]: {value} is not a view of {part_id}; it has {names}.") + if value not in targets: + targets.append(value) + already = [_view_name(i, records[i]) for i in targets if records[i].get("dimensioned")] + if already: + raise ValueError( + f"views: {', '.join(already)} already dimensioned; each measurement appears once " + "(ISO 129-1), so dimensioning it again would stack a duplicate on every measurement." + ) + return targets + + async def dimension_part( backend, part_id: str, @@ -549,47 +786,88 @@ async def dimension_part( style: str = "chain", hole_table_threshold: int = 8, fits: dict | None = None, + views: list[int] | None = None, ) -> dict: - """Dimension every view of a part, laid out so no two texts overlap.""" + """Dimension the views of a part that carry no dimensions yet, laid out without overlaps. + + ``views`` names view indices (the order `read_part` lists them in); by + default every view not yet dimensioned is. A view already dimensioned is + never dimensioned again (ISO 129-1), and the flag that says so is written + back into the part payload. + """ payload = await read_part(backend, part_id) part = build_part(payload["part"]) - records = payload.get("views") or [] + records = list(payload.get("views") or []) + if not records: + raise ValueError(f"{part_id}: the payload carries no view to dimension.") if fits is not None and not isinstance(fits, dict): raise ValueError( f"fits: expected a dict of {{key: ISO 286 code}}, got {type(fits).__name__}." ) fits = dict(fits or {}) + targets = _dimension_targets(part_id, records, views) + origin, rotation = _placement(payload) + threshold = int(hole_table_threshold) + + # Everything that can be refused is refused before the first DIMENSION is + # drawn: a fit no target view claims must not leave half a view dimensioned + # and unflagged behind it. + planned: list[tuple[int, View, list]] = [] unclaimed = set(fits) + for index in targets: + view = _view_from_record(part, payload, records[index]) + mine = _fits_for_view(part, view, fits, style=style, threshold=threshold) + unclaimed -= set(mine) + intents = dimension_intents( + part, view, style=style, hole_table_threshold=threshold, fits=mine or None + ) + planned.append((index, view, layout_dimensions(view, intents))) + if unclaimed: + # No view claimed these keys. Hand them back to the module that owns the + # vocabulary so the refusal names the keys that do exist, and the hole + # table's own message survives. + dimension_intents( + part, + planned[0][1], + style=style, + hole_table_threshold=threshold, + fits={key: fits[key] for key in sorted(unclaimed)}, + ) handles: list[str] = [] resolved_rows: list[dict] = [] skipped: list[dict] = [] table_handle: str | None = None - for record in records: - view = _view_from_record(part, payload, record) - mine = _fits_for_view(part, view, fits, style=style, threshold=int(hole_table_threshold)) - unclaimed -= set(mine) - intents = dimension_intents( - part, view, style=style, hole_table_threshold=hole_table_threshold, fits=mine or None - ) - placed = layout_dimensions(view, intents) + for index, view, placed in planned: + record = records[index] offset = record["offset"] for intent in placed: - moved = intent._replace( + frame = intent._replace( p1=(intent.p1[0] + offset[0], intent.p1[1] + offset[1]), p2=(intent.p2[0] + offset[0], intent.p2[1] + offset[1]), text_at=(intent.text_at[0] + offset[0], intent.text_at[1] + offset[1]), ) - handle, row = await _draw_dimension(backend, moved) + angle = None + if intent.kind == "linear": + # the measuring direction is decided in the part's own frame and + # turned with it; the rotated points alone no longer show it + horizontal = abs(frame.p2[1] - frame.p1[1]) <= abs(frame.p2[0] - frame.p1[0]) + angle = (0.0 if horizontal else 90.0) + rotation + moved = frame._replace( + p1=_turn(frame.p1, origin, rotation), + p2=_turn(frame.p2, origin, rotation), + text_at=_turn(frame.text_at, origin, rotation), + ) + handle, row = await _draw_dimension(backend, moved, linear_angle=angle) if handle is None: skipped.append(row) continue handles.append(handle) resolved_rows.append(row) - rows = hole_table_rows(part, view, hole_table_threshold=hole_table_threshold) - if rows and table_handle is None: + rows = hole_table_rows(part, view, hole_table_threshold=threshold) + if rows and table_handle is None and not payload.get("hole_table"): info = await backend.entity_create_table( record["bbox"][2] + DEFAULT_GAP, record["bbox"][3], @@ -603,18 +881,26 @@ async def dimension_part( ) table_handle = info.handle handles.append(info.handle) + if placed or rows: + record["dimensioned"] = True + else: + # Said, not silently counted as done: the view stays unflagged, so a + # later call can dimension it once the dimension layer covers it. + skipped.append( + { + "view": _view_name(index, record), + "kind": view.kind, + "skipped": f"the dimension layer emits no intents for a {view.kind} view " + "in this build; the view is left undimensioned", + } + ) - if unclaimed: - # No view claimed these keys. Hand them back to the module that owns the - # vocabulary so the refusal names the keys that do exist, and the hole - # table's own message survives. - dimension_intents( - part, - _view_from_record(part, payload, records[0]), - style=style, - hole_table_threshold=hole_table_threshold, - fits={key: fits[key] for key in sorted(unclaimed)}, - ) + payload["views"] = records + if table_handle: + payload["hole_table"] = table_handle + await backend.entity_set_xdata( + part_id, APP_ID, to_values({k: v for k, v in payload.items() if k != "part_id"}) + ) return { "part_id": part_id, @@ -623,6 +909,7 @@ async def dimension_part( "resolved": resolved_rows, "skipped": skipped, "hole_table": table_handle, + "views_dimensioned": [i for i in targets if records[i].get("dimensioned")], "backend": backend.name, } @@ -735,12 +1022,21 @@ async def draw_from_spec(backend, spec: dict) -> dict: part = build_part(item.get("part") or {}) except ValueError as exc: raise ValueError(f"parts[{index}]: {exc}") from exc - at = item.get("at") or [0.0, 0.0] + at = item.get("at") + if at is None: + at = [0.0, 0.0] + if isinstance(at, (str, bytes)) or not isinstance(at, (list, tuple)) or len(at) != 2: + raise ValueError(f"parts[{index}]: at: expected [x, y], got {at!r}.") jobs.append( { "part": part, - "at": (float(at[0]), float(at[1])), - "rotation": float(item.get("rotation", 0.0)), + "at": ( + _number(at[0], f"parts[{index}]: at[0]", positive=False), + _number(at[1], f"parts[{index}]: at[1]", positive=False), + ), + "rotation": _number( + item.get("rotation", 0.0), f"parts[{index}]: rotation", positive=False + ), "views": tuple(item.get("views") or ("front",)), "projection": str(item.get("projection", "first")), "dimension": bool(item.get("dimension", True)), @@ -759,7 +1055,14 @@ async def draw_from_spec(backend, spec: dict) -> dict: begun = await backend.transaction_begin() if not isinstance(begun, dict) or not begun.get("ok"): - raise RuntimeError(f"mech_part_from_spec: could not open a transaction: {begun}") + # a refusal, not a crash: ValueError is what the tool layer turns into + # a ToolError, and nothing has been drawn yet + detail = begun.get("error") if isinstance(begun, dict) else begun + raise ValueError( + f"mech_part_from_spec: could not open a transaction ({detail}); nothing was drawn. " + "If one is already open, close it with transaction_commit or " + "transaction_rollback and retry." + ) drawn: list[dict] = [] try: diff --git a/server.py b/server.py index 0707d87..0871291 100644 --- a/server.py +++ b/server.py @@ -8161,13 +8161,25 @@ async def mech_part_draw( ), ], at_x: Annotated[ - float, Field(default=0.0, description="WCS X of the first view's lower-left") + float, + Field( + default=0.0, + description="WCS X of the first view's lower-left corner (the rotation pivot)", + ), ] = 0.0, at_y: Annotated[ - float, Field(default=0.0, description="WCS Y of the first view's lower-left") + float, + Field( + default=0.0, + description="WCS Y of the first view's lower-left corner (the rotation pivot)", + ), ] = 0.0, rotation: Annotated[ - float, Field(default=0.0, description="Degrees CCW applied to every view") + float, + Field( + default=0.0, + description="Degrees CCW; turns every view and its dimensions about (at_x, at_y)", + ), ] = 0.0, views: Annotated[ list[str] | None, @@ -8274,7 +8286,14 @@ async def mech_view_add( float, Field(default=20.0, description="Gap between this view and its parent (mm)") ] = 20.0, scale: Annotated[ - float, Field(default=1.0, description="View scale recorded on the view") + float, + Field( + default=1.0, + description=( + "Must be 1.0: views are drawn full size (a detail takes its scale in " + "detail.scale; a sheet scale is the viewport's)" + ), + ), ] = 1.0, ctx: Context = None, ) -> dict: @@ -8344,20 +8363,34 @@ async def mech_dimension_part( description="ISO 286 fit per feature id, e.g. {'bearing_seat': 'k6', 'bore': 'H7'}", ), ] = None, + views: Annotated[ + list[int] | None, + Field( + default=None, + description=( + "View indices to dimension, in mech_part_inspect order " + "(default: every view not yet dimensioned)" + ), + ), + ] = None, ctx: Context = None, ) -> dict: """Every dimension the part and its features ask for, laid out without overlaps. Axial lengths are chained, stacked from a baseline or measured from an origin; diameters stack outside the silhouette; fillets get a radius. Each - measurement appears once (ISO 129-1). `fits` resolves an ISO 286 code + measurement appears once (ISO 129-1): every view record carries a + `dimensioned` flag, so after a `mech_part_draw` that dimensioned the front + view and a `mech_view_add`, this call dimensions only the new view, and + `views` naming a view already dimensioned is refused by name. `fits` resolves an ISO 286 code against the measured nominal through the repository's authored tables and appends the code to the dimension text; an explicit `tol` on a feature's own intent does the same through the ISO 129 path. Refused by name: a fit together with an explicit tolerance, a fit outside the authored ISO 286 range (over 1 mm to 500 mm, IT4-IT11), a fit on an - intent with no measurable nominal, an unknown style, an unknown part_id, and + intent with no measurable nominal, an unknown style, an unknown part_id, a + call when every view is already dimensioned (the message lists them), and `part_id=None` on a drawing that holds more than one part. An angular intent is reported in `skipped` rather than drawn, because a DimIntent carries no vertex. @@ -8374,6 +8407,7 @@ async def mech_dimension_part( style=style, hole_table_threshold=hole_table_threshold, fits=fits, + views=views, ) except ValueError as exc: raise ToolError(str(exc)) from exc diff --git a/tests/test_mech_draw.py b/tests/test_mech_draw.py index 6340e2b..800290b 100644 --- a/tests/test_mech_draw.py +++ b/tests/test_mech_draw.py @@ -281,3 +281,214 @@ async def test_draw_from_spec_rolls_back_when_a_draw_fails_midway(backend, monke monkeypatch.undo() assert not await entities(backend, type_filter="POINT", layer=ANCHOR_LAYER) assert not await entities(backend, type_filter="LINE") + + +# -- review round: rotation, single dimensioning, honest refusals ------------- + +TWO_STEP = { + "kind": "revolved", + "name": "shaft", + "material": "steel", + "segments": [{"length": 20.0, "d_outer": 25.0}, {"length": 40.0, "d_outer": 30.0}], + "features": [], +} + +TUBE = { + "kind": "revolved", + "name": "tube", + "material": "steel", + "segments": [{"length": 30.0, "d_outer": 60.0, "d_inner": 40.0}], + "features": [], +} + + +def _geometry_bbox(backend): + xs: list[float] = [] + ys: list[float] = [] + for e in backend._doc.modelspace(): + if e.dxftype() == "LINE": + xs += [e.dxf.start.x, e.dxf.end.x] + ys += [e.dxf.start.y, e.dxf.end.y] + return [min(xs), min(ys), max(xs), max(ys)] + + +def _defpoints(backend): + return sorted( + (round(e.dxf.get(name).x, 6), round(e.dxf.get(name).y, 6)) + for e in backend._doc.modelspace().query("DIMENSION") + for name in ("defpoint2", "defpoint3") + if e.dxf.hasattr(name) + ) + + +def _turn90(point, about): + return (round(about[0] - (point[1] - about[1]), 6), round(about[1] + (point[0] - about[0]), 6)) + + +async def test_rotation_turns_geometry_record_and_dimensions_about_at(backend): + """rotation=90 about at=(100, 100): the part, its recorded box and every + DIMENSION defpoint are the rotation=0 drawing turned about `at`.""" + from backends.ezdxf_backend import EzdxfBackend + + flat = EzdxfBackend() + await flat.connect() + await flat.drawing_new() + try: + await draw_part(flat, build_part(TWO_STEP), at=(100.0, 100.0), rotation=0.0) + expected = sorted(_turn90(p, (100.0, 100.0)) for p in _defpoints(flat)) + finally: + await flat.disconnect() + + result = await draw_part(backend, build_part(TWO_STEP), at=(100.0, 100.0), rotation=90.0) + geometry = _geometry_bbox(backend) + assert geometry == pytest.approx([70.0, 100.0, 100.0, 172.0]) + assert result["views"][0]["bbox"] == pytest.approx(geometry) + read = await read_part(backend, result["part_id"]) + assert read["views"][0]["bbox"] == pytest.approx(geometry) + assert read["views"][0]["frame_bbox"] == pytest.approx([100.0, 100.0, 172.0, 130.0]) + assert expected + assert _defpoints(backend) == expected + + +async def test_a_view_added_to_a_rotated_part_turns_with_it(backend): + drawn = await draw_part( + backend, build_part(TWO_STEP), at=(0.0, 0.0), rotation=90.0, dimension=False + ) + added = await add_view(backend, drawn["part_id"], "side") + # first angle: the side view sits left of the front in the part's frame, + # which is BELOW it once the sheet is turned 90 degrees CCW + front, side = drawn["views"][0]["bbox"], added["view"]["bbox"] + assert side[3] <= front[1] + 1e-6 + + +async def test_draw_then_add_view_then_dimension_never_duplicates(backend): + drawn = await draw_part(backend, build_part(SHAFT), at=(0.0, 0.0), dimension=True) + before = len(await entities(backend, type_filter="DIMENSION")) + assert before == drawn["dimensions"] > 0 + await add_view(backend, drawn["part_id"], "side") + result = await dimension_part(backend, drawn["part_id"]) + assert result["views_dimensioned"] == [1] + after = len(await entities(backend, type_filter="DIMENSION")) + assert after == before + result["count"] + assert result["count"] > 0 + + with pytest.raises(ValueError, match="already dimensioned"): + await dimension_part(backend, drawn["part_id"]) + with pytest.raises(ValueError, match=r"0 \(front\) already dimensioned"): + await dimension_part(backend, drawn["part_id"], views=[0]) + assert len(await entities(backend, type_filter="DIMENSION")) == after + + +async def test_dimension_part_twice_is_refused_not_stacked(backend): + drawn = await draw_part(backend, build_part(PLATE), at=(0.0, 0.0), dimension=False) + first = await dimension_part(backend, drawn["part_id"]) + with pytest.raises(ValueError, match="every view is already dimensioned"): + await dimension_part(backend, drawn["part_id"]) + assert len(await entities(backend, type_filter="DIMENSION")) == first["count"] + + +async def test_a_view_the_dimension_layer_cannot_dimension_is_reported_not_flagged(backend): + drawn = await draw_part(backend, build_part(SHAFT), at=(0.0, 0.0)) + await add_view( + backend, + drawn["part_id"], + "detail", + detail={"center": [20.0, 12.5], "radius": 8.0, "scale": 5.0, "label": "D"}, + ) + result = await dimension_part(backend, drawn["part_id"]) + assert result["views_dimensioned"] == [] + assert any(row.get("kind") == "detail" for row in result["skipped"]) + assert result["count"] == 0 + + +async def test_an_island_that_does_not_attach_removes_the_hatch_and_raises(backend, monkeypatch): + drawn = await draw_part(backend, build_part(TUBE), at=(0.0, 0.0), dimension=False) + real = backend.hatch_add_boundary + + async def refuse(handle, edges): + return await real(handle, []) + + monkeypatch.setattr(backend, "hatch_add_boundary", refuse) + with pytest.raises(ValueError, match="did not attach"): + await add_view( + backend, + drawn["part_id"], + "section", + plane={"p1": [15.0, -40.0], "p2": [15.0, 40.0], "label": "B"}, + ) + assert not await entities(backend, type_filter="HATCH") + + +async def test_an_engine_without_hatch_edge_paths_refuses_before_drawing(backend, monkeypatch): + from backends.base import CapabilityMap, FeatureCapability + from backends.capability import UnsupportedCapabilityError + + drawn = await draw_part(backend, build_part(TUBE), at=(0.0, 0.0), dimension=False) + count = len(await entities(backend)) + monkeypatch.setattr( + backend, + "capabilities", + lambda: CapabilityMap( + "com", {"hatch_edge_paths": FeatureCapability(False, reason="measured")} + ), + ) + with pytest.raises(UnsupportedCapabilityError) as excinfo: + await add_view( + backend, + drawn["part_id"], + "section", + plane={"p1": [15.0, -40.0], "p2": [15.0, 40.0], "label": "B"}, + ) + assert excinfo.value.capability == "hatch_edge_paths" + assert len(await entities(backend)) == count + + +async def test_a_scale_that_would_only_be_recorded_is_refused(backend): + with pytest.raises(ValueError, match="full size"): + await draw_part(backend, build_part(PLATE), scale=0.5) + drawn = await draw_part(backend, build_part(PLATE), dimension=False) + with pytest.raises(ValueError, match="full size"): + await add_view(backend, drawn["part_id"], "side", scale=2.0) + + +async def test_a_second_added_view_steps_along_its_own_projection_axis(backend): + drawn = await draw_part( + backend, build_part(PLATE), at=(0.0, 0.0), dimension=False, projection="third" + ) + side = await add_view(backend, drawn["part_id"], "side") + section = await add_view( + backend, + drawn["part_id"], + "section", + plane={"p1": [0.0, 25.0], "p2": [80.0, 25.0], "label": "A"}, + ) + front = drawn["views"][0]["bbox"] + # third angle puts both to the RIGHT; the second steps further right, + # never back onto the front view + assert section["view"]["bbox"][0] >= side["view"]["bbox"][2] + 20.0 - 1e-6 + assert section["view"]["bbox"][0] > front[2] + + +async def test_a_top_view_sharing_the_x_range_is_not_shoved(backend): + drawn = await draw_part(backend, build_part(PLATE), at=(0.0, 0.0), dimension=False) + await add_view(backend, drawn["part_id"], "side") + top = await add_view(backend, drawn["part_id"], "top", gap=25.0) + front = drawn["views"][0]["bbox"] + assert top["view"]["bbox"][0] == pytest.approx(front[0]) + assert top["view"]["bbox"][2] == pytest.approx(front[2]) + + +@pytest.mark.parametrize("at", [[], [5.0], {"x": 1}, "0,0", [1.0, "a"]]) +async def test_a_malformed_at_is_refused_with_its_part_path(backend, at): + with pytest.raises(ValueError, match=r"parts\[0\]: at"): + await draw_from_spec(backend, {"parts": [{"part": PLATE, "at": at}]}) + assert not await entities(backend, type_filter="POINT", layer=ANCHOR_LAYER) + + +async def test_a_transaction_that_cannot_open_is_a_value_error(backend, monkeypatch): + async def busy(): + return {"ok": False, "error": "a transaction is already open"} + + monkeypatch.setattr(backend, "transaction_begin", busy) + with pytest.raises(ValueError, match="could not open a transaction"): + await draw_from_spec(backend, {"parts": [{"part": PLATE, "at": [0.0, 0.0]}]})