From 64cec37cc70d13ac8df12043ebd5e008f4b8c109 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Thu, 17 Sep 2026 15:10:00 +0300 Subject: [PATCH] =?UTF-8?q?fix(task-22):=20ucs=5Flist=20decides=20"world"?= =?UTF-8?q?=20from=20the=20frame,=20not=20the=20empty=20name=20=E2=80=94?= =?UTF-8?q?=20an=20unnamed=20UCS=20is=20its=20own=20current=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An unnamed UCS (UCS Origin / 3P without saving, the common kind) has an empty UCSNAME exactly like WCS does, so both engines answered `current: world` while the drawing was not in WCS — verified live on AutoCAD 2026 (`_.UCS _O 10,10,0` → UCSNAME="" WORLDUCS=0) and on the DXF it writes ($UCSNAME='' $UCSORG=(0,20,0)). A silent wrong value. - COM: a named match still wins; otherwise WORLDUCS (AutoCAD's own answer) decides, and WORLDUCS=0 appends a `name: None` row built from UCSORG/UCSXDIR/UCSYDIR. - ezdxf: the same rule against $UCSORG/$UCSXDIR/$UCSYDIR via the new `engineering.environment.ucs.is_world_axes`; a stale $UCSNAME falls back to the frame instead of leaving no row current. - Server `ucs_list` result: `current` is null for an unnamed UCS (never `world`), plus `current_unnamed: bool` so null is unambiguous. - Tests: helper, headless (unnamed row, stale name, and the exact AutoCAD DXF over the wire), and COM fake reading WORLDUCS + the frame variables. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- backends/com_backend.py | 28 ++++++-- backends/contracts/environment.py | 9 ++- backends/ezdxf_backend.py | 35 ++++++--- engineering/environment/ucs.py | 28 ++++++++ server.py | 12 +++- tests/test_views_ucs.py | 116 +++++++++++++++++++++++++++++- 6 files changed, 209 insertions(+), 19 deletions(-) diff --git a/backends/com_backend.py b/backends/com_backend.py index 1594725..391f00c 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -1733,7 +1733,7 @@ class ComBackend(AutoCADBackend): } async def ucs_list(self) -> list[dict]: - from engineering.environment.ucs import WORLD + from engineering.environment.ucs import WORLD, WORLD_ORIGIN, WORLD_X_AXIS, WORLD_Y_AXIS def _sync(): app = _acad_app() @@ -1742,15 +1742,33 @@ class ComBackend(AutoCADBackend): rows = [ { "name": WORLD, - "origin": [0.0, 0.0, 0.0], - "x_axis": [1.0, 0.0, 0.0], - "y_axis": [0.0, 1.0, 0.0], - "current": current == "", + "origin": list(WORLD_ORIGIN), + "x_axis": list(WORLD_X_AXIS), + "y_axis": list(WORLD_Y_AXIS), + "current": False, } ] collection = doc.UserCoordinateSystems for index in range(int(collection.Count)): rows.append(self._com_ucs_row(collection.Item(index), current)) + if any(row["current"] for row in rows[1:]): + return rows + # No saved entry is current. UCSNAME is empty for an unnamed UCS + # (UCS Origin / 3P without saving) as well as for WCS — verified + # live (AutoCAD 2026: after `_.UCS _O 10,10,0`, UCSNAME="" and + # WORLDUCS=0). WORLDUCS is AutoCAD's own answer to "is this WCS". + if int(app.GetVariable("WORLDUCS")) == 1: + rows[0]["current"] = True + else: + rows.append( + { + "name": None, + "origin": [float(c) for c in app.GetVariable("UCSORG")], + "x_axis": [float(c) for c in app.GetVariable("UCSXDIR")], + "y_axis": [float(c) for c in app.GetVariable("UCSYDIR")], + "current": True, + } + ) return rows return await self._run(_sync) diff --git a/backends/contracts/environment.py b/backends/contracts/environment.py index 24e8f70..dd0f5bc 100644 --- a/backends/contracts/environment.py +++ b/backends/contracts/environment.py @@ -86,7 +86,14 @@ class EnvironmentContract(ABC): @abstractmethod async def ucs_list(self) -> list[dict]: - """→ ``[{"name", "origin", "x_axis", "y_axis", "current": bool}]`` + the implicit "world".""" + """→ ``[{"name", "origin", "x_axis", "y_axis", "current": bool}]`` + the implicit "world". + + Exactly one row is current. An *unnamed* current UCS (``UCS Origin`` / + ``3P`` without saving) is a trailing row with ``name: None`` carrying + its frame — "world" is current only when the frame is the WCS + (WORLDUCS live, the ``$UCS*`` header headless), never because the + name is empty. + """ ... @abstractmethod diff --git a/backends/ezdxf_backend.py b/backends/ezdxf_backend.py index 287e3e9..92fa017 100644 --- a/backends/ezdxf_backend.py +++ b/backends/ezdxf_backend.py @@ -2692,22 +2692,41 @@ class EzdxfBackend(AutoCADBackend): } async def ucs_list(self) -> list[dict]: - from engineering.environment.ucs import WORLD + from engineering.environment.ucs import ( + WORLD, + WORLD_ORIGIN, + WORLD_X_AXIS, + WORLD_Y_AXIS, + is_world_axes, + ) def _sync(): doc = self._require_doc() - current = str(doc.header.get("$UCSNAME", "") or "") - rows = [self._ucs_row(WORLD, (0, 0, 0), (1, 0, 0), (0, 1, 0), current == "")] + header = doc.header + current = str(header.get("$UCSNAME", "") or "") + origin = tuple(header.get("$UCSORG", WORLD_ORIGIN)) + x_axis = tuple(header.get("$UCSXDIR", WORLD_X_AXIS)) + y_axis = tuple(header.get("$UCSYDIR", WORLD_Y_AXIS)) + rows = [self._ucs_row(WORLD, WORLD_ORIGIN, WORLD_X_AXIS, WORLD_Y_AXIS, False)] + named_current = False for ucs in doc.ucs: + is_current = current != "" and ucs.dxf.name.lower() == current.lower() + named_current = named_current or is_current rows.append( self._ucs_row( - ucs.dxf.name, - ucs.dxf.origin, - ucs.dxf.xaxis, - ucs.dxf.yaxis, - current != "" and ucs.dxf.name.lower() == current.lower(), + ucs.dxf.name, ucs.dxf.origin, ucs.dxf.xaxis, ucs.dxf.yaxis, is_current ) ) + if named_current: + return rows + # No saved entry is current. An empty $UCSNAME is NOT "world": an + # unnamed UCS (UCS Origin / 3P without saving) has an empty name + # too, and AutoCAD writes exactly that. The frame decides, the way + # WORLDUCS does live. + if is_world_axes(origin, x_axis, y_axis): + rows[0]["current"] = True + else: + rows.append(self._ucs_row(None, origin, x_axis, y_axis, True)) return rows return await self._async(_sync) diff --git a/engineering/environment/ucs.py b/engineering/environment/ucs.py index 6314729..3e95e7b 100644 --- a/engineering/environment/ucs.py +++ b/engineering/environment/ucs.py @@ -12,6 +12,34 @@ import math WORLD = "world" ORTHO_TOLERANCE_DEG = 1e-3 +#: The WCS frame — what ``$UCSORG`` / ``$UCSXDIR`` / ``$UCSYDIR`` (and the +#: UCSORG / UCSXDIR / UCSYDIR system variables) hold while no UCS is active. +WORLD_ORIGIN = (0.0, 0.0, 0.0) +WORLD_X_AXIS = (1.0, 0.0, 0.0) +WORLD_Y_AXIS = (0.0, 1.0, 0.0) +WORLD_TOLERANCE = 1e-9 + + +def is_world_axes(origin, x_axis, y_axis, *, tol: float = WORLD_TOLERANCE) -> bool: + """True when the frame *is* the WCS — the test ``$UCSNAME == ""`` cannot make. + + An unnamed UCS (``UCS Origin`` / ``UCS 3P`` without saving — the common + kind) also has an empty name, so "is the drawing in WCS" has to be + answered from the frame itself: AutoCAD's own answer is WORLDUCS, and + this is the same comparison for a header that has no such variable. + """ + for got, want in ( + (origin, WORLD_ORIGIN), + (x_axis, WORLD_X_AXIS), + (y_axis, WORLD_Y_AXIS), + ): + coords = tuple(float(c) for c in got) + if len(coords) != 3: + return False + if any(abs(a - b) > tol for a, b in zip(coords, want, strict=True)): + return False + return True + def _vec3(value, where: str) -> tuple[float, float, float]: if not isinstance(value, (list, tuple)) or len(value) != 3: diff --git a/server.py b/server.py index cbdb451..0c34a30 100644 --- a/server.py +++ b/server.py @@ -7050,13 +7050,19 @@ async def view_named_list(ctx: Context = None) -> dict: ) async def ucs_list(ctx: Context = None) -> dict: """The `world` row first, then every UCS table entry with origin and unit - axes, `current` on the active one. Tool coordinates stay WCS on both - engines whatever is current. No refusals. Pack: settings · lean: no.""" + axes, `current` on the active one. `world` is current only when the frame + IS the WCS (WORLDUCS live, the `$UCS*` header headless) — an unnamed UCS + (`UCS Origin` / `3P` without saving) has an empty name too and is + reported as a trailing row with `name: null` and its origin/axes, so + `current` is `null` then and never `world`. Tool coordinates stay WCS on + both engines whatever is current. No refusals. Pack: settings · lean: no.""" rows = await _backend(ctx).ucs_list() + active = next((r for r in rows if r["current"]), None) return { "ucs": rows, "count": len(rows), - "current": next((r["name"] for r in rows if r["current"]), None), + "current": active["name"] if active else None, + "current_unnamed": bool(active and active["name"] is None), } diff --git a/tests/test_views_ucs.py b/tests/test_views_ucs.py index 6943f85..7d36b7f 100644 --- a/tests/test_views_ucs.py +++ b/tests/test_views_ucs.py @@ -12,8 +12,10 @@ from __future__ import annotations import types import pytest +from fastmcp import Client -from engineering.environment.ucs import resolve_ucs_axes +import server +from engineering.environment.ucs import is_world_axes, resolve_ucs_axes from engineering.environment.views import resolve_view_args pytestmark = pytest.mark.asyncio @@ -53,6 +55,14 @@ def test_ucs_axes_are_normalised_and_orthogonality_is_measured(): resolve_ucs_axes([0, 0], [1, 0, 0], [0, 1, 0]) +def test_world_is_decided_by_the_frame_not_the_name(): + assert is_world_axes((0, 0, 0), (1, 0, 0), (0, 1, 0)) + assert is_world_axes([0.0, 0.0, 1e-12], [1.0, 0.0, 0.0], [0.0, 1.0, 0.0]) + assert not is_world_axes((0, 20, 0), (1, 0, 0), (0, 1, 0)) # UCS Origin + assert not is_world_axes((0, 0, 0), (0, 1, 0), (-1, 0, 0)) # rotated 90° + assert not is_world_axes((0, 0), (1, 0, 0), (0, 1, 0)) + + # ── headless: named views ─────────────────────────────────────────────────── @@ -213,6 +223,74 @@ async def test_ucs_replace_keeps_one_entry(backend): assert len(list(backend._doc.ucs)) == 1 +async def test_ucs_list_does_not_call_an_unnamed_ucs_world(backend): + """``UCS Origin`` / ``UCS 3P`` without saving — the common kind — leaves + ``$UCSNAME`` empty exactly like WCS does (AutoCAD 2026 writes + ``$UCSNAME=''``, ``$UCSORG=(0,20,0)`` after ``_.UCS _O``). The frame, not + the name, says whether the drawing is in WCS.""" + await backend.ucs_set("FRONT", [10, 10, 0], [0, 1, 0], [-1, 0, 0]) + header = backend._doc.header + header["$UCSNAME"] = "" + header["$UCSORG"] = (50.0, 20.0, 0.0) + header["$UCSXDIR"] = (0.0, 1.0, 0.0) + header["$UCSYDIR"] = (-1.0, 0.0, 0.0) + rows = await backend.ucs_list() + assert [r["name"] for r in rows] == ["world", "FRONT", None] + assert [r["current"] for r in rows] == [False, False, True] + assert rows[2] == { + "name": None, + "origin": [50.0, 20.0, 0.0], + "x_axis": [0.0, 1.0, 0.0], + "y_axis": [-1.0, 0.0, 0.0], + "current": True, + } + # ...and the same empty name with the WCS frame really is world. + await backend.ucs_restore("world") + rows = await backend.ucs_list() + assert [r["name"] for r in rows] == ["world", "FRONT"] + assert rows[0]["current"] is True and rows[1]["current"] is False + + +async def test_ucs_list_a_stale_name_falls_back_to_the_frame(backend): + """``$UCSNAME`` naming an entry the table no longer holds is not a + current row; the frame decides between world and unnamed.""" + header = backend._doc.header + header["$UCSNAME"] = "GONE" + rows = await backend.ucs_list() + assert [(r["name"], r["current"]) for r in rows] == [("world", True)] + header["$UCSORG"] = (0.0, 20.0, 0.0) + rows = await backend.ucs_list() + assert [(r["name"], r["current"]) for r in rows] == [("world", False), (None, True)] + assert rows[1]["origin"] == [0.0, 20.0, 0.0] + + +async def test_ucs_list_tool_reports_an_unnamed_ucs_over_the_wire(monkeypatch, tmp_path): + """The exact file AutoCAD writes after ``_.UCS _O 0,20,0`` without saving: + ``$UCSNAME=''`` and ``$UCSORG=(0,20,0)``. The tool used to answer + ``current: "world"``.""" + import ezdxf + + doc = ezdxf.new("R2018") + doc.ucs.add( + "FRONT", dxfattribs={"origin": (10, 10, 0), "xaxis": (0, 1, 0), "yaxis": (-1, 0, 0)} + ) + doc.header["$UCSNAME"] = "" + doc.header["$UCSORG"] = (0.0, 20.0, 0.0) + path = tmp_path / "unnamed_ucs.dxf" + doc.saveas(str(path)) + + monkeypatch.setenv("AUTOCAD_MCP_BACKEND", "ezdxf") + async with Client(server.mcp) as client: + await client.call_tool("drawing_open", {"path": str(path)}) + payload = (await client.call_tool("ucs_list", {})).structured_content + assert payload["current"] is None and payload["current_unnamed"] is True + assert payload["count"] == 3 + assert [r["name"] for r in payload["ucs"]] == ["world", "FRONT", None] + assert payload["ucs"][0]["current"] is False + assert payload["ucs"][2]["current"] is True + assert payload["ucs"][2]["origin"] == [0.0, 20.0, 0.0] + + # ── live engine, against a fake ActiveX surface ────────────────────────────── @@ -311,7 +389,14 @@ def com_backend(monkeypatch): from backends import com_backend as module document = _FakeDocument() - variables = {"CMDACTIVE": 0, "UCSNAME": ""} + variables = { + "CMDACTIVE": 0, + "UCSNAME": "", + "WORLDUCS": 1, + "UCSORG": (0.0, 0.0, 0.0), + "UCSXDIR": (1.0, 0.0, 0.0), + "UCSYDIR": (0.0, 1.0, 0.0), + } app = types.SimpleNamespace(GetVariable=lambda name: variables[name]) monkeypatch.setattr(module, "_acad_doc", lambda: document) monkeypatch.setattr(module, "_acad_app", lambda: app) @@ -372,6 +457,33 @@ async def test_com_ucs_add_uses_points_on_the_axes_not_vectors(com_backend): } +async def test_com_ucs_list_reads_worlducs_not_the_empty_name(com_backend): + """Verified live on AutoCAD 2026: after ``_.UCS _O 10,10,0`` (unnamed), + ``UCSNAME=""`` and ``WORLDUCS=0`` — the same empty name WCS has.""" + backend, document, variables = com_backend + await backend.ucs_set("FRONT", [10, 10, 0], [0, 1, 0], [-1, 0, 0]) + variables.update( + { + "UCSNAME": "", + "WORLDUCS": 0, + "UCSORG": (0.0, 20.0, 0.0), + "UCSXDIR": (1.0, 0.0, 0.0), + "UCSYDIR": (0.0, 1.0, 0.0), + } + ) + rows = await backend.ucs_list() + assert [(r["name"], r["current"]) for r in rows] == [ + ("world", False), + ("FRONT", False), + (None, True), + ] + assert rows[2]["origin"] == [0.0, 20.0, 0.0] and rows[2]["x_axis"] == [1.0, 0.0, 0.0] + # WORLDUCS=1 with the same empty name is world, and no unnamed row appears. + variables.update({"WORLDUCS": 1, "UCSORG": (0.0, 0.0, 0.0)}) + rows = await backend.ucs_list() + assert [(r["name"], r["current"]) for r in rows] == [("world", True), ("FRONT", False)] + + async def test_com_ucs_restore_world_sends_the_ucs_command_when_idle(com_backend): backend, document, variables = com_backend result = await backend.ucs_restore("world")