From d072df9fd444800da0a3e2b291d91b82156d7490 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Tue, 22 Sep 2026 12:37:33 +0300 Subject: [PATCH] fix(task-24a): apply_standard decides each unit write on a read taken after the dimension style switch f7528b2 moved the five unit reads of apply_standard ahead of the style stage so a refused read is a refusal with nothing written -- and then decided the writes on those reads. Making a dimension style current restores every DIM* variable from the style (ezdxf: _sync_header_to_dimstyle on dimstyle_set_current; AutoCAD: ActiveDimStyle), so on a drawing whose ISO-25/ANSI already carried another scale the gate read (1.0) equalled the target, the DIMSCALE write was skipped and the drawing ended at the style's scale while the report said `changed: {}` -- against the spec's "LTSCALE 1, DIMSCALE 1" and the tool docstring. The pre-f7528b2 loop ran after the switch and got this right; this is a regression of f7528b2. The gate read stays (readable before anything is written); each variable is read again after the style stage and the write is decided on that value, so `changed` says what the write moved. Measured on the running AutoCAD 2026 (25.1s, 2026-09-22, scratch Documents.Add(), Close(False), real ComBackend after connect()): ISO-25 modified to DIMSCALE 2.0 (dimstyle_modify -> {'DIMSCALE': [1.0, 2.0]}), Annotative current, doc.GetVariable('DIMSCALE') -> 1.0 before: apply_standard(iso, layers=False) -> {'changed': {}, 'applied': True}; ActiveDimStyle ISO-25; doc.GetVariable('DIMSCALE') -> 2.0 after: -> {'changed': {'DIMSCALE': [2.0, 1.0]}, 'applied': True}; ActiveDimStyle ISO-25; doc.GetVariable('DIMSCALE') -> 1.0 ezdxf, same setup (dimstyle_create ISO-25 {DIMSCALE: 2.0}, Standard current): before `{'INSUNITS': [6, 4]}` and DIMSCALE 2.0 afterwards; after `{'INSUNITS': [6, 4], 'DIMSCALE': [2.0, 1.0]}` and DIMSCALE 1.0. Tests: one regression per engine (both fail on the f7528b2 apply.py); test_com_apply_standard_creates_both_styles_and_sets_units now pins the gate reads first *and* the re-read-then-write sequence after CopyFrom, instead of enshrining the stale-read order. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/standards/apply.py | 18 +++++--- tests/test_styles.py | 80 +++++++++++++++++++++++++++++++--- 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/engineering/standards/apply.py b/engineering/standards/apply.py index 6e87880..6c40138 100644 --- a/engineering/standards/apply.py +++ b/engineering/standards/apply.py @@ -68,17 +68,23 @@ async def apply_standard( written. (The units loop used to run after both styles; on the live engine its INSUNITS read went through a member the ActiveX Application does not have, so the call failed with ISO-25 and ISOCP already created - and current.) + and current.) That first read is only the gate: making a dimension style + current restores every DIM* variable from the style (ezdxf syncs the + header; AutoCAD 2026 measured ``ActiveDimStyle = ISO-25`` with a style + DIMSCALE of 2.0 moving the document's DIMSCALE 1.0 -> 2.0), so each + variable is read again *after* the style switch and the write is decided + on that value. Deciding on the gate read skipped the DIMSCALE write + whenever a pre-existing ISO-25/ANSI carried another scale, and reported + ``changed: {}`` on a drawing left at the style's scale. """ key, row = resolve_standard(standard) if not isinstance(layers, bool) or not isinstance(units, bool): raise TypeError("layers and units must be booleans") - current: dict[str, Any] = {} - if units: + if units: # the refusal gate: every variable readable before anything is written for var, _value in STANDARD_SYSVARS: try: - current[var] = await backend.system_get_variable(var) + await backend.system_get_variable(var) except Exception as exc: raise RuntimeError( f"apply_standard: could not read {var} before writing " @@ -104,7 +110,9 @@ async def apply_standard( changed: dict[str, list] = {} if units: for var, value in STANDARD_SYSVARS: - old = current[var] + # Re-read after the style switch: the gate value is stale for + # DIM* once another dimension style has been made current. + old = await backend.system_get_variable(var) if _same(old, value): continue await backend.system_set_variable(var, value) diff --git a/tests/test_styles.py b/tests/test_styles.py index 1a5aece..3d9c9ff 100644 --- a/tests/test_styles.py +++ b/tests/test_styles.py @@ -1692,6 +1692,28 @@ async def test_second_apply_is_idempotent_and_reports_nothing_created(backend): assert backend._doc.header["$DIMSTYLE"] == "ISO-25" +async def test_apply_writes_dimscale_1_when_the_existing_style_restores_another(backend): + """Regression: the units were decided on a read taken *before* the style + switch. Making ISO-25 current restores its DIMSCALE into the header, so a + pre-existing ISO-25 at scale 2 (any modified company template) left the + drawing at 2.0 while the report said ``changed: {}`` -- the spec promises + DIMSCALE 1. The gate read stays; the write is decided on a read after + the switch.""" + from engineering.standards.apply import apply_standard + + await backend.dimstyle_create("ISO-25", {"DIMSCALE": 2.0}) + await backend.dimstyle_set_current("Standard") + assert await backend.system_get_variable("DIMSCALE") == 1.0 + result = await apply_standard(backend, "iso", layers=False) + assert result["dimstyle"] == {"name": "ISO-25", "created": False, "current": True} + assert result["settings"]["changed"] == {"INSUNITS": [6, 4], "DIMSCALE": [2.0, 1.0]} + assert await backend.system_get_variable("DIMSCALE") == 1.0 + assert backend._doc.header["$DIMSTYLE"] == "ISO-25" + # The style itself is used as found (dimstyle_modify changes one on purpose). + iso = next(r for r in await backend.dimstyle_list() if r["name"] == "ISO-25") + assert iso["values"]["DIMSCALE"] == 2.0 + + async def test_apply_refuses_an_unknown_standard_before_writing(backend): from engineering.standards.apply import apply_standard @@ -1726,17 +1748,32 @@ async def test_com_apply_standard_creates_both_styles_and_sets_units(com_backend assert result["dimstyle"]["created"] is True and result["textstyle"]["created"] is True assert result["settings"]["changed"] == {"INSUNITS": [1, 4], "LUNITS": [4, 2]} names = [call[0] for call in document.calls] + unit_vars = ["INSUNITS", "LUNITS", "AUNITS", "LTSCALE", "DIMSCALE"] assert names[:5] == ["GetVariable"] * 5, "every unit variable is read on the document first" - assert [c[1] for c in document.calls[:5]] == [ - "INSUNITS", - "LUNITS", - "AUNITS", - "LTSCALE", - "DIMSCALE", - ] + assert [c[1] for c in document.calls[:5]] == unit_vars, "the gate: readable before any write" assert names[5] == "TextStyles.Add" and "DimStyles.Add" in names assert ("ActiveTextStyle", "ISOCP") in document.calls assert document.ActiveDimStyle.Name == "ISO-25" + # The writes are decided on a *second* read taken after the dimension + # style stage (ActiveDimStyle restores the style's DIM* variables), never + # on the gate read. The style stage ends with CopyFrom plus its report + # reads; the units stage then re-reads each variable in turn and writes + # only the ones that moved (the second read of a written variable is + # ``system_set_variable``'s own old-value read). + style_end = names.index("CopyFrom") + units_start = document.calls.index(("GetVariable", "INSUNITS"), style_end) + assert units_start > style_end + assert [c for c in document.calls[units_start:] if c[1] in unit_vars] == [ + ("GetVariable", "INSUNITS"), + ("GetVariable", "INSUNITS"), + ("SetVariable", "INSUNITS", 4), + ("GetVariable", "LUNITS"), + ("GetVariable", "LUNITS"), + ("SetVariable", "LUNITS", 2), + ("GetVariable", "AUNITS"), + ("GetVariable", "LTSCALE"), + ("GetVariable", "DIMSCALE"), + ] assert document.variables["DIMDSEP"] == "," and document.variables["INSUNITS"] == 4 @@ -1752,6 +1789,35 @@ def _record_reads(document): document.GetVariable = _get +async def test_com_apply_standard_writes_dimscale_1_after_the_style_switch_restores_2( + com_backend, +): + """Regression, measured live on AutoCAD 2026 (scratch Documents.Add()): + ISO-25 modified to DIMSCALE 2.0, Annotative current at 1.0 -> + ``apply_standard`` returned ``{'changed': {}, 'applied': True}`` and the + document read DIMSCALE 2.0 afterwards, because ``ActiveDimStyle = ISO-25`` + restored the style's DIMSCALE after the gate read had seen 1.0. The write + is decided on a read after the switch; the report says what moved.""" + from engineering.standards.apply import apply_standard + + backend, document = com_backend + document.variables.update({"INSUNITS": 4, "LUNITS": 2, "AUNITS": 0, "LTSCALE": 1.0}) + await backend.dimstyle_create("ISO-25", {"DIMSCALE": 2.0}) # not current: Standard stays + assert document.ActiveDimStyle.Name == "Standard" and document.variables["DIMSCALE"] == 1.0 + document.calls.clear() + _record_reads(document) + result = await apply_standard(backend, "iso", layers=False) + assert result["dimstyle"] == {"name": "ISO-25", "created": False, "current": True} + assert result["settings"]["changed"] == {"DIMSCALE": [2.0, 1.0]} + assert document.ActiveDimStyle.Name == "ISO-25" + assert document.variables["DIMSCALE"] == 1.0, "the drawing is at the spec's DIMSCALE 1" + switch = document.calls.index(("ActiveDimStyle", "ISO-25")) + tail = document.calls[switch + 1 :] + assert ("GetVariable", "DIMSCALE") in tail and ("SetVariable", "DIMSCALE", 1.0) in tail + assert tail.index(("GetVariable", "DIMSCALE")) < tail.index(("SetVariable", "DIMSCALE", 1.0)) + assert not any(c[0] == "CopyFrom" for c in document.calls), "the style is used as found" + + async def test_com_apply_standard_refuses_before_writing_when_a_sysvar_read_fails(com_backend): """Regression: the units loop ran *after* both styles were created, and its INSUNITS read went through ``AcadApplication.GetVariable`` -- a member the