From cdff63fce0a54c789806a73107528f3999e117ef Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Thu, 24 Sep 2026 12:24:02 +0300 Subject: [PATCH] fix(understand): a DWG the operator has open is exported as it stands, never closed On the live engine, take_snapshot read a .dwg path by calling Documents.Open(path, read-only), exporting and then Close(False) on what Open returned. If the operator already had that drawing open, that was their document, and closing it unsaved could throw away unsaved work. drawing_understand, drawing_scale_check, drawing_diff and the takeoffs all read a path through this function. The review of drawing_diff raised the risk and did not probe it live, to avoid a modal dialog on the shared AutoCAD. The fix makes the question moot: the path is looked up among the open documents first, case-blind as Windows paths are, and an open drawing is exported as it stands (unsaved edits included) and left open. Only a document opened here is closed here. The new test, against the fake that behaves as measured, fails on the old code with an open and a close in the log. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/understand/snapshot.py | 32 ++++++++++++++++++++++---- tests/test_understand_snapshot.py | 37 +++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/engineering/understand/snapshot.py b/engineering/understand/snapshot.py index 76cab08..2e9d6a0 100644 --- a/engineering/understand/snapshot.py +++ b/engineering/understand/snapshot.py @@ -33,7 +33,9 @@ Where the snapshot comes from: ``drawing_understand``'s docstring says so. A DWG path on the live engine is opened read-only, exported the same way and - closed without saving. + closed without saving - unless AutoCAD already has that drawing open: then + its open document is exported as it stands and left open, because closing + it could throw away the operator's unsaved work. Records hold what the readers need and nothing else: WCS points (OCS is translated here with ``backends/ocs.py``, so a mirrored ARC or LWPOLYLINE is @@ -330,6 +332,21 @@ def read_snapshot(path: str) -> Snapshot: return _read(str(target), source=str(target)) +def _already_open(app, path: str): + """The document AutoCAD already has open at ``path``, or None. Opens nothing.""" + wanted = os.path.normcase(os.path.abspath(path)) + documents = app.Documents + for index in range(int(documents.Count)): + doc = documents.Item(index) + try: + full = str(doc.FullName or "") + except Exception: # a document closing under us answers nothing; it is not ours + continue + if full and os.path.normcase(os.path.abspath(full)) == wanted: + return doc + return None + + def _com_export(backend, path: str | None) -> tuple[str, str, str]: """(snapshot file, its temporary folder, source) - runs on the COM thread.""" from backends import com_backend as com @@ -339,8 +356,13 @@ def _com_export(backend, path: str | None) -> tuple[str, str, str]: if path is None: doc = com._acad_doc() else: - opened = app.Documents.Open(str(path), True) # read-only - doc = opened + # A drawing the operator already has open is exported as it stands and + # left open: opening it again and closing "it" unsaved could close their + # copy with its unsaved work. Only a document opened here is closed here. + doc = _already_open(app, str(path)) + if doc is None: + opened = app.Documents.Open(str(path), True) # read-only + doc = opened folder = tempfile.mkdtemp(prefix="acadmcp_snap_") try: name = str(backend._wait_out_rejected_call(lambda: doc.Name)) @@ -373,7 +395,9 @@ async def take_snapshot(backend, path: str | None = None) -> Snapshot: ``path`` given: a ``.dxf`` is read directly (on either engine); a ``.dwg`` needs the live engine, which opens it read-only, exports it and closes it - - headlessly it is refused with the ``dwg`` capability. ``path`` None: the + or, when AutoCAD already has it open, exports that document as it stands + (unsaved edits included) and leaves it open - and headlessly it is refused + with the ``dwg`` capability. ``path`` None: the headless engine reads its in-memory document under its own lock; the live engine exports the active document to a temporary DXF (see the module docstring for what was measured) and deletes it afterwards. The snapshot of diff --git a/tests/test_understand_snapshot.py b/tests/test_understand_snapshot.py index 2150d62..a9a56b2 100644 --- a/tests/test_understand_snapshot.py +++ b/tests/test_understand_snapshot.py @@ -257,8 +257,9 @@ class _Selections: class _Doc: - def __init__(self, name, log): + def __init__(self, name, log, full_name=""): self.Name = name + self.FullName = full_name # "" for a drawing never saved, as AutoCAD answers self.log = log self.SelectionSets = _Selections(log) @@ -276,8 +277,16 @@ class _Doc: class _Documents: - def __init__(self, log): + def __init__(self, log, open_docs=()): self.log = log + self.open_docs = list(open_docs) + + @property + def Count(self): # noqa: N802 + return len(self.open_docs) + + def Item(self, index): # noqa: N802 + return self.open_docs[index] def Open(self, path, read_only): # noqa: N802 self.log.append(("open", Path(path).name, read_only)) @@ -285,11 +294,11 @@ class _Documents: class _App: - def __init__(self, log): - self.Documents = _Documents(log) + def __init__(self, log, open_docs=()): + self.Documents = _Documents(log, open_docs) -def _live(monkeypatch, log): +def _live(monkeypatch, log, open_docs=()): backend = ComBackend() async def run_inline(func, *args, **kwargs): # bypass the STA executor @@ -301,7 +310,7 @@ def _live(monkeypatch, log): backend._run = run_inline backend._ensure_document_state = ensure_state active = _Doc("Drawing1.dwg", log) - monkeypatch.setattr(cb, "_acad_app", lambda: _App(log)) + monkeypatch.setattr(cb, "_acad_app", lambda: _App(log, open_docs)) monkeypatch.setattr(cb, "_acad_doc", lambda: active) return backend, active @@ -342,3 +351,19 @@ async def test_a_dwg_on_the_live_engine_is_opened_read_only_and_closed_unsaved( assert ("open", "plant.dwg", True) in log assert ("close", False) in log assert log[-1] == ("ensure_document_state",) + + +@pytest.mark.asyncio +async def test_a_dwg_already_open_is_read_from_its_document_and_never_closed(monkeypatch, tmp_path): + # Opening a drawing AutoCAD already has open, then closing "it" unsaved, could + # close the operator's copy and lose its unsaved work. The open document is + # exported as it stands and left open: the server closes only what it opened. + path = tmp_path / "plant.dwg" + path.write_bytes(b"AC1032") + log: list[tuple] = [] + theirs = _Doc("plant.dwg", log, full_name=str(path)) + backend, _active = _live(monkeypatch, log, open_docs=[_Doc("Drawing1.dwg", log), theirs]) + snap = await take_snapshot(backend, str(path)) + assert snap.source == "live:plant.dwg" + assert ("export", "snap", "DXF") in log + assert not [event for event in log if event[0] in ("open", "close")]