From f6951e2597eac6e241f426e84dca174d91602113 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Mon, 21 Sep 2026 21:36:07 +0300 Subject: [PATCH] fix(task-18): the COM plot is foreground, by layout name, and confirmed on disk; plot_style_list reads PrinterStyleSheetPath Three review findings against SECTION 19 on the live engine, each measured on AutoCAD 2026 in a scratch document with the operator's BACKGROUNDPLOT=2: - plot_style_list read Preferences.Files.PrintStyleSheetPath; the ActiveX member is PrinterStyleSheetPath, so the AttributeError was swallowed and every row came back installed=None. Renamed in the backend, the server docstring and the test fake (which faked the same wrong name). Live: 13 rows, all 9 catalogue ctb files installed, four .stb files as installed. - ComBackend.drawing_export_pdf returned ok before any file existed under background plotting, so batch_plot died on path.stat() with an unhandled FileNotFoundError and a later foreground plot raised E_FAIL. The export now sets BACKGROUNDPLOT=0 around PlotToFile (restored after), honours the method's bool and reports ok only with the file on disk (bytes included). batch_plot turns a backend RuntimeError or a phantom ok into a per-row ok:false, keeping the rows already plotted. - With a sheet current, the Model row was that sheet plotted again: the COM export switched tabs only when a layout was named. layout=None now makes Model current (restoring the tab), and batch_plot names every layout, Model included, so both engines plot the tab the row is labelled with. Live: Model row /Title (Model) at 210x297 beside the A3 sheet, the active tab and BACKGROUNDPLOT restored. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- backends/com_backend.py | 82 ++++++++++++--- engineering/standards/plot.py | 26 ++++- server.py | 2 +- tests/test_batch_plot.py | 189 +++++++++++++++++++++++++++++++++- 4 files changed, 278 insertions(+), 21 deletions(-) diff --git a/backends/com_backend.py b/backends/com_backend.py index 0967b7d..256aa22 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -1041,23 +1041,73 @@ class ComBackend(AutoCADBackend): return await self.drawing_save_as(path, "dxf") async def drawing_export_pdf(self, path: str, layout: str | None = None) -> dict: + """Plot one layout to PDF through ``Plot.PlotToFile`` and confirm the file. + + ``layout=None`` means model space, as the tool advertises — the Model + tab is made current for the plot (and the previous tab restored), so + the row is never silently the sheet that happened to be active. + + Measured on AutoCAD 2026 with the operator's default BACKGROUNDPLOT=2: + ``PlotToFile`` returns True immediately, the job is queued to the + background plotter, and no file exists when the call comes back — + seconds later the folder was still empty, and a foreground plot + attempted after such a stranded job raised E_FAIL. So the plot is + forced to the foreground (``BACKGROUNDPLOT=0``, restored afterwards), + the method's boolean is honoured, and ``ok`` is reported only once the + file is on disk with a size. + """ + def _sync(): doc = _acad_doc() - previous = None - if layout: - previous = doc.ActiveLayout.Name - if previous != layout: - doc.ActiveLayout = doc.Layouts.Item(layout) + target = layout.strip() if layout and layout.strip() else "Model" + previous = doc.ActiveLayout.Name + switched = False + if previous.lower() != target.lower(): + doc.ActiveLayout = doc.Layouts.Item(target) + switched = True + old_background = None try: - plot = doc.Plot - plot.PlotToFile(path, "DWG To PDF.pc3") - result = {"ok": True, "path": path} - if layout: - result["layout"] = layout - return result + try: + old_background = int(doc.GetVariable("BACKGROUNDPLOT")) + except Exception as exc: + log.debug("BACKGROUNDPLOT unreadable (%s); plotting as configured", exc) + if old_background not in (None, 0): + doc.SetVariable("BACKGROUNDPLOT", 0) + plotted = doc.Plot.PlotToFile(path, "DWG To PDF.pc3") + written = Path(path) + if not plotted: + return { + "ok": False, + "path": path, + "layout": target, + "error": "PlotToFile returned False: AutoCAD refused the plot " + "(check the layout's plot area and the 'DWG To PDF.pc3' device)", + } + if not written.is_file(): + return { + "ok": False, + "path": path, + "layout": target, + "error": "PlotToFile reported success but wrote no file at " + f"{path} (BACKGROUNDPLOT was {old_background})", + } + return { + "ok": True, + "path": path, + "layout": target, + "bytes": written.stat().st_size, + } finally: - if previous is not None and previous != layout: - doc.ActiveLayout = doc.Layouts.Item(previous) + if old_background not in (None, 0): + try: + doc.SetVariable("BACKGROUNDPLOT", old_background) + except Exception as exc: + log.warning("could not restore BACKGROUNDPLOT=%s: %s", old_background, exc) + if switched: + try: + doc.ActiveLayout = doc.Layouts.Item(previous) + except Exception as exc: + log.warning("could not restore the active layout %r: %s", previous, exc) return await self._run(_sync) @@ -2267,7 +2317,7 @@ class ComBackend(AutoCADBackend): async def plot_style_list(self) -> list[dict]: """The ctb catalogue, marked against the files in AutoCAD's plot style path. - ``Preferences.Files.PrintStyleSheetPath`` is a ``;``-separated list of + ``Preferences.Files.PrinterStyleSheetPath`` is a ``;``-separated list of folders. Every read is guarded: a folder that cannot be listed is skipped, and when the Preferences object is unreachable the catalogue rows come back with ``installed: None`` rather than a fabricated False. @@ -2277,9 +2327,9 @@ class ComBackend(AutoCADBackend): def _sync(): rows = [{"name": name, "source": "catalog", "installed": None} for name in CTB_CATALOG] try: - raw = str(_acad_app().Preferences.Files.PrintStyleSheetPath or "") + raw = str(_acad_app().Preferences.Files.PrinterStyleSheetPath or "") except Exception as exc: - log.debug("Preferences.Files.PrintStyleSheetPath unreadable: %s", exc) + log.debug("Preferences.Files.PrinterStyleSheetPath unreadable: %s", exc) return rows installed: dict[str, str] = {} for folder in (part.strip() for part in raw.split(";")): diff --git a/engineering/standards/plot.py b/engineering/standards/plot.py index ac035c5..bec01ef 100644 --- a/engineering/standards/plot.py +++ b/engineering/standards/plot.py @@ -36,7 +36,10 @@ async def batch_plot( """Plot layouts to PDF and read each sheet size back from the file. ``layouts=None`` plots every paper-space layout; ``Model`` is plotted only - when named. Refuses (``ValueError``, before any PDF is written): an empty + when named, and is passed to the engine by name so the model row is model + space whichever tab is current. A sheet the engine cannot plot, or reports + plotted without a file, is a per-row ``ok: False`` with its ``error``; the + rows before it stay. Refuses (``ValueError``, before any PDF is written): an empty list, a layout that does not exist, a pattern with a path separator, and a pattern without ``{layout}`` when more than one sheet would be written. The output folder is created, then every output path goes through @@ -95,12 +98,31 @@ async def batch_plot( sheets: list[dict] = [] for name, path in planned: is_model = name.lower() == "model" - result = await backend.drawing_export_pdf(str(path), layout=None if is_model else name) + # The layout is always named — "Model" included — so the engine plots + # the tab this row is labelled with, never the one that happens to be + # current. A plot that fails inside the engine (COM raises RuntimeError + # for an AutoCAD error) or that reports ok without leaving a file is a + # failed row, not a lost batch: the sheets already plotted are kept. + try: + result = await backend.drawing_export_pdf(str(path), layout=name) + except RuntimeError as exc: + sheets.append({"layout": name, "ok": False, "path": str(path), "error": str(exc)}) + continue if not result.get("ok", False): sheets.append( {"layout": name, "ok": False, "path": str(path), "error": result.get("error")} ) continue + if not path.is_file(): + sheets.append( + { + "layout": name, + "ok": False, + "path": str(path), + "error": "the engine reported the plot ok but no file was written", + } + ) + continue row = {"layout": name, "ok": True, "path": str(path), "bytes": path.stat().st_size} try: width, height = read_mediabox(str(path)) diff --git a/server.py b/server.py index 1200130..990e6eb 100644 --- a/server.py +++ b/server.py @@ -6917,7 +6917,7 @@ async def page_setup_apply( async def plot_style_list(ctx: Context = None) -> dict: """The ctb files AutoCAD ships (`monochrome.ctb`, `acad.ctb`, `Grayscale.ctb`, the Screening set, …) and, on the live engine, the files actually present - in `Preferences.Files.PrintStyleSheetPath` (`source: "installed"`, + in `Preferences.Files.PrinterStyleSheetPath` (`source: "installed"`, `installed: true/false` per row). Headlessly `installed` is `null`: there is no installation to scan, and the catalogue is not evidence of one. Never refuses; an unreadable Preferences object degrades to the catalogue. diff --git a/tests/test_batch_plot.py b/tests/test_batch_plot.py index 91e8631..7af9a84 100644 --- a/tests/test_batch_plot.py +++ b/tests/test_batch_plot.py @@ -1,13 +1,15 @@ """batch_plot: every sheet to PDF, each sheet size read back from the PDF. The composition is exercised on the headless engine end to end; the COM-only -piece of this task (the installed ctb scan) is fake-tested; the SECTION 19 +pieces of this task (the installed ctb scan, and the plot call's shape — +foreground, by layout name, confirmed on disk) are fake-tested; the SECTION 19 tools are called directly with a minimal context. """ from __future__ import annotations import types +from pathlib import Path import pytest from fastmcp.exceptions import ToolError @@ -100,6 +102,70 @@ async def test_a_fresh_drawing_plots_its_one_default_sheet_at_the_dxf_default_si assert row["path"].endswith("Layout1.pdf") and row["paper"] == "ISO_A3" +async def test_model_is_named_to_the_engine_even_when_a_sheet_is_current(backend, tmp_path): + """The Model row must be model space whichever tab is current. On COM the + export used to plot ``ActiveLayout`` for ``layout=None``, so with a sheet + current the "Model" PDF was that sheet again; batch_plot now names the + layout for every row, Model included.""" + pytest.importorskip("matplotlib", reason="rendering needs the [pdf] extra") + await _two_sheets(backend) + await backend.layout_set_current("A3") + seen: list[str | None] = [] + original = backend.drawing_export_pdf + + async def _recording(path, layout=None): + seen.append(layout) + return await original(path, layout=layout) + + backend.drawing_export_pdf = _recording + result = await batch_plot(backend, ["Model", "A3"], str(tmp_path)) + assert seen == ["Model", "A3"] + assert result["ok"] and result["sheets"][0]["paper"] is None + assert result["sheets"][1]["paper"] == "ISO_A3" + + +async def test_a_sheet_the_engine_cannot_plot_is_a_failed_row_not_a_lost_batch(backend, tmp_path): + """COM raises RuntimeError for an AutoCAD error (E_FAIL after a stranded + background job, say). That is one row's failure; the sheets already + written stay in the report with their evidence.""" + pytest.importorskip("matplotlib", reason="rendering needs the [pdf] extra") + await _two_sheets(backend) + original = backend.drawing_export_pdf + + async def _flaky(path, layout=None): + if layout == "B": + raise RuntimeError("AutoCAD COM error (-0x7ffdfff7): Unspecified error") + return await original(path, layout=layout) + + backend.drawing_export_pdf = _flaky + result = await batch_plot(backend, None, str(tmp_path)) + assert result["ok"] is False and result["count"] == 2 + a3, b = result["sheets"] + assert a3["ok"] and a3["paper"] == "ISO_A3" + assert b == { + "layout": "B", + "ok": False, + "path": str(tmp_path / "untitled-B.pdf"), + "error": "AutoCAD COM error (-0x7ffdfff7): Unspecified error", + } + + +async def test_a_plot_reported_ok_without_a_file_is_a_failed_row(backend, tmp_path): + """A background plot answers ok before any file exists; the row says so + instead of dying on ``path.stat()`` with an unhandled FileNotFoundError.""" + await _two_sheets(backend) + + async def _phantom(path, layout=None): + return {"ok": True, "path": path} + + backend.drawing_export_pdf = _phantom + result = await batch_plot(backend, ["A3"], str(tmp_path)) + assert result["ok"] is False + row = result["sheets"][0] + assert row["ok"] is False and "no file" in row["error"] + assert "bytes" not in row and "mediabox_mm" not in row + + # ── the tools ─────────────────────────────────────────────────────────────── @@ -159,7 +225,7 @@ def com_backend(monkeypatch, tmp_path): (styles / "readme.txt").write_bytes(b"") app = types.SimpleNamespace( Preferences=types.SimpleNamespace( - Files=types.SimpleNamespace(PrintStyleSheetPath=f"{styles};C:\\does\\not\\exist") + Files=types.SimpleNamespace(PrinterStyleSheetPath=f"{styles};C:\\does\\not\\exist") ) ) monkeypatch.setattr(module, "_acad_app", lambda: app) @@ -199,3 +265,122 @@ async def test_com_plot_style_list_survives_an_unreachable_preferences_object( rows = await backend.plot_style_list() assert [row["name"] for row in rows] == list(CTB_CATALOG) assert all(row["installed"] is None for row in rows) + + +# ── COM: the plot call's shape ────────────────────────────────────────────── + + +@pytest.fixture +def com_plot(monkeypatch): + """A fake document whose ``Plot.PlotToFile`` records the active layout and + BACKGROUNDPLOT at the moment of the plot — the two things AutoCAD acts on + and the reviewer measured wrong: with a sheet current, ``layout=None`` + plotted that sheet; with the operator's BACKGROUNDPLOT=2 the file did not + exist when the call returned.""" + pytest.importorskip("win32com.client", reason="pywin32 not installed") + from backends import com_backend as module + + state = { + "vars": {"BACKGROUNDPLOT": 2}, + "sets": [], + "active": "Layout1", + "plots": [], + "write_file": True, + "plotted": True, + } + names = ["Model", "Layout1", "Layout2"] + + class _Layout: + def __init__(self, name): + self.Name = name + + class _Layouts: + def Item(self, key): + for name in names: + if name.lower() == str(key).lower(): + return _Layout(name) + raise KeyError(key) + + class _Plot: + def PlotToFile(self, path, config): + state["plots"].append((state["active"], config, state["vars"]["BACKGROUNDPLOT"])) + if state["write_file"]: + Path(path).write_bytes(b"%PDF-1.4 fake") + return state["plotted"] + + class _Doc: + Layouts = _Layouts() + Plot = _Plot() + + @property + def ActiveLayout(self): + return _Layout(state["active"]) + + @ActiveLayout.setter + def ActiveLayout(self, layout): + state["active"] = layout.Name + + def GetVariable(self, name): + return state["vars"][name] + + def SetVariable(self, name, value): + state["sets"].append((name, value)) + state["vars"][name] = value + + monkeypatch.setattr(module, "_acad_doc", lambda: _Doc()) + backend = module.ComBackend() + + async def _run_inline(func, *args, **kwargs): + return func(*args, **kwargs) + + monkeypatch.setattr(backend, "_run", _run_inline) + return backend, state + + +async def test_com_export_pdf_plots_model_space_by_name_in_the_foreground(com_plot, tmp_path): + backend, state = com_plot + out = tmp_path / "model.pdf" + result = await backend.drawing_export_pdf(str(out)) + assert state["plots"] == [("Model", "DWG To PDF.pc3", 0)], ( + "layout=None is model space, plotted with BACKGROUNDPLOT forced to 0" + ) + assert result == {"ok": True, "path": str(out), "layout": "Model", "bytes": out.stat().st_size} + assert state["active"] == "Layout1", "the sheet that was current is current again" + assert state["vars"]["BACKGROUNDPLOT"] == 2, "the operator's setting is restored" + assert state["sets"] == [("BACKGROUNDPLOT", 0), ("BACKGROUNDPLOT", 2)] + + +async def test_com_export_pdf_plots_the_named_sheet_and_restores_the_tab(com_plot, tmp_path): + backend, state = com_plot + result = await backend.drawing_export_pdf(str(tmp_path / "l2.pdf"), layout="layout2") + assert [row[0] for row in state["plots"]] == ["Layout2"] + assert result["ok"] and result["layout"] == "layout2" + assert state["active"] == "Layout1" + + +async def test_com_export_pdf_leaves_backgroundplot_alone_when_already_foreground( + com_plot, tmp_path +): + backend, state = com_plot + state["vars"]["BACKGROUNDPLOT"] = 0 + await backend.drawing_export_pdf(str(tmp_path / "m.pdf")) + assert state["sets"] == [] + + +async def test_com_export_pdf_reports_a_refused_plot(com_plot, tmp_path): + backend, state = com_plot + state["plotted"] = False + state["write_file"] = False + result = await backend.drawing_export_pdf(str(tmp_path / "no.pdf"), layout="Layout1") + assert result["ok"] is False and "PlotToFile returned False" in result["error"] + assert state["vars"]["BACKGROUNDPLOT"] == 2 + + +async def test_com_export_pdf_reports_a_plot_that_left_no_file(com_plot, tmp_path): + """PlotToFile answering True with nothing on disk is the background-plot + signature; it is a failure, not an ok.""" + backend, state = com_plot + state["write_file"] = False + result = await backend.drawing_export_pdf(str(tmp_path / "gone.pdf")) + assert result["ok"] is False and "wrote no file" in result["error"] + assert "bytes" not in result