Files
autocad-mcp-pro/docs/analysis/01-DIAGNOSTICS.md
T
Umutcan Edizsalan 0c7f64ceb6 fix(correctness-bundle): I16 correctness fixes + dead code removal + ruff clean
N1: COM hatch non-associative (AddHatch associativity=False)
N2: ezdxf refuses DWG saves with clear error; server.py derives fmt from extension
R19: dimension_aligned dist→distance; dimension_angular correct add_angular_dim_2l args (base/line1/line2/location)
R9: polar array 360° no-duplicate (divisor=n when fill_angle%360<ε)
R13: entity_offset side_x/side_y honored in ezdxf via cross-product (LINE) and distance-to-center (CIRCLE)
R18: drawing_new bootstrap failure surfaces status="degraded" instead of swallowing silently
Dead code: engineering/section.py, gear.generate_tooth_profile, CommandResult, _ACI_NAMES, set_layer_active, ensure_linetypes, ComBackend._batch_mode instance attr, engineering/__init__ build-seam comment
Ruff: 38 auto-fixed + 5 manual → codebase is now ruff-clean; CI lint is now blocking

184 tests pass.
2026-06-18 11:34:30 +03:00

310 lines
32 KiB
Markdown

<!-- Generated by the DIAGNOSTICS workflow (wf_c3e5770d-2cf), 2026-06-18.
Verified: 31 | Confirmed: 28 | Refuted/fixed: 2 | New from sweep: 17.
NOTE: R7/R8 (sanitizer wiring + LISP-aliasing/path-traversal defensive tests) were
NOT auto-verified — the security subagents hit a provider cyber-safety filter.
They are tracked for manual defensive review during the hardening work item. -->
# DIAGNOSTICS Report — AutoCAD MCP Pro
Synthesized from the verified recon risk register + fresh-sweep findings. REFUTED / ALREADY-FIXED items are excluded from the ranked body and listed in the appendix. Duplicates between the two sources are merged. Line numbers reflect **current** code (verified against the working tree).
---
## CRITICAL
_None._
---
## HIGH
### N1 — COM `entity_create_hatch` builds an ASSOCIATIVE hatch then deletes its boundary polyline
- **Severity:** HIGH
- **Location:** `backends/com_backend.py:659-673`
- **Description:** `mspace.AddHatch(0, pattern, True)` requests an *associative* hatch linked to the appended boundary loop, but the code then calls `bnd_pline.Delete()` (`com_backend.py:673`) on that very polyline. In live AutoCAD this either errors on `Delete`, drops the boundary, or leaves a hatch whose boundary tracking breaks on the next regen/edit.
- **Fix:** Change the 3rd arg to non-associative — `AddHatch(0, pattern, False)` — so the computed fill survives deleting the throwaway boundary. If associativity is truly wanted, keep the boundary polyline instead of deleting it.
### N2 — `drawing_save_as` default format diverges (ezdxf `"dxf"` vs COM/ABC `"dwg"`); `drawing_finalize` relies on the default
- **Severity:** HIGH (merges register **R10**/**R11** with the fresh-sweep duplicate)
- **Location:** ABC default `backends/base.py:158` (`"dwg"`); ezdxf `backends/ezdxf_backend.py:346` (`"dxf"`); COM `backends/com_backend.py:516` (`"dwg"`); consumed with no fmt at `server.py:2299` (`drawing_finalize` → `backend.drawing_save_as(str(validated))`).
- **Description:** `drawing_finalize` passes no `fmt`, so each backend's default decides the output. Two distinct real defects layer here:
1. **ezdxf silently mislabels DWG (R11):** `drawing_save_as` *ignores* `fmt` entirely — `doc.saveas(path)` derives format from the extension — yet still returns `{"format": fmt}`. For `fmt="dwg"`/`"dwt"` it writes DXF tag-stream bytes (ezdxf has no DWG/DWT writer) while echoing `format:"dwg"`. (`ezdxf_backend.py:346-353`)
2. **COM writes DWG content under a `.dxf` name (R10):** COM's `SaveAs(path, fmt_map[fmt])` honors `fmt`, *not* the extension, so a `.dxf` save_path under COM yields DWG bytes in a `.dxf`-named file.
- Net: identical `drawing_finalize(save_path=...)` produces different on-disk formats per backend, decoupled from the extension the user supplied.
- **Fix:** In `drawing_finalize`, derive `fmt` from the `save_path` extension and pass it explicitly. Unify the ABC + both impl signatures to one default. In ezdxf, reject `fmt not in {dxf, dwt}` (raise `ToolError` for `dwg`) instead of writing DXF and echoing `format=fmt`.
---
## MED
### R4 — `dim_overlap` critique focus is a guaranteed silent no-op (no backend populates the keys it reads)
- **Severity:** MED (claimed HIGH; downgraded — silently returns `[]`, no incorrect output)
- **Location:** reads `engineering/critique.py:199-201`; extractors `backends/ezdxf_backend.py:167-169` and `backends/com_backend.py:204-244` (verified at `critique.py:199-201`).
- **Description:** `_check_dim_overlap` reads `text_position`/`defpoint`/`insertion_point`, but the ezdxf DIM branch sets only `props["dim_type"]` and COM dims get only a generic `bounding_box`. None of the three keys is produced anywhere, so `ip` is always falsy, `dim_points` stays empty, and the focus always returns `[]`.
- **Fix:** Populate a dim reference point (text midpoint / defpoint) in both `_entity_info` extractors for DIM types, OR fall back to `properties["bounding_box"]` center in `_check_dim_overlap`.
### R9 — ezdxf `entity_array_polar` overlaps the original on full-circle fill and diverges from COM
- **Severity:** MED
- **Location:** `backends/ezdxf_backend.py:947-948`
- **Description:** `step = radians(fill_angle) / max(count-1, 1)` with loop `range(1, count)` places the last copy at `step*(count-1) = 360°` for a full ring — coincident with the original at 0°. Also diverges from COM (`com_backend.py:921` uses AutoCAD `ArrayPolar`, spacing `360/count`): `count=4, fill=360` → COM 90/180/270 vs ezdxf 120/240/360. `count-1` is only correct for partial-arc fills.
- **Fix:** `divisor = count if abs(fill_angle % 360) < eps else max(count-1, 1)`.
### R12 — ezdxf `dimension_angular` ignores `tx/ty` and hardcodes `distance=10.0`
- **Severity:** MED
- **Location:** `backends/ezdxf_backend.py:706-723` (hardcode at `:715`)
- **Description:** Accepts `tx/ty` (advertised as "text position X/Y" at `server.py:721-722`) but calls `add_angular_dim_2l(..., distance=10.0)`, ignoring them. COM honors `tx/ty` via `AddDimAngular` (`com_backend.py:770-784`), so identical inputs place the arc differently per backend; the fixed 10.0 mislocates the arc for very large/small parts.
- **Fix:** Pass `location=(tx,ty)` to `add_angular_dim_2l`, or compute `distance=hypot(tx-vx, ty-vy)`.
### R13 / N2-offset — `entity_offset` ignores `side_x/side_y` in BOTH backends; side semantics diverge; ezdxf supports only LINE/CIRCLE
- **Severity:** MED (merges register **R13** with the two fresh-sweep offset findings)
- **Location:** ezdxf `backends/ezdxf_backend.py:869-909`; COM `backends/com_backend.py:861-875`; ABC sig `backends/base.py:315-319`; tool `server.py:859-868`.
- **Description:** Both backends accept `side_x/side_y` and never read them (dead params; the tool advertises them as "a point on the offset side"). ezdxf hard-fixes the LINE normal `Vec2(-dir.y, dir.x)*d` (`:882`) and CIRCLE `radius+d` (`:896`); every other type raises `RuntimeError("Offset not supported …")` (`:908`) — ARC/LWPOLYLINE/POLYLINE/ELLIPSE/SPLINE unsupported, contradicting the tool's "line, circle, or polyline" claim. COM delegates to `ent.Offset(distance)`, whose sign/side convention is not guaranteed to match ezdxf's fixed left-normal — same `(handle, distance)` can offset to opposite sides per backend.
- **Fix:** Honor `side_x/side_y` in both backends (pick the copy whose ref point is nearest the side point, or sign `distance` from the cross product against the offset direction). In ezdxf, support ARC/LWPOLYLINE/POLYLINE/ELLIPSE or drop the documented polyline/side claims. Reconcile so positive distance means the same side in both engines.
### R15 — `system_about` `tool_groups` static dict omits ~20 registered tools and misfiles one
- **Severity:** MED
- **Location:** `server.py:2070-2106`
- **Description:** The hand-maintained `tool_groups` dict omits `drawing_close`, `entity_trim/extend/fillet/chamfer`, all 7 engineering tools, and all 8 premium tools (~15+ shown), and lists `entity_delete_many` under `entity_creation` though it's tagged `{entity, modify}` with `destructiveHint=True`. Live `total_tools` stays correct; only the per-group breakdown drifts.
- **Fix:** Derive `tool_groups` dynamically from registered tools' tags, or at minimum add the engineering/premium groups, move `entity_delete_many`, and add the 5 missing modify/drawing tools.
### R18 — `drawing_new` bootstrap failures return as success
- **Severity:** MED
- **Location:** `server.py:282-293`; per-item swallow at `engineering/layers.py:90-91, 128-129`
- **Description:** Bootstrap is wrapped in `try/except` that stores `{"error": …}` and still `return result` representing a created drawing; per-item failures are caught internally as `"failed: {exc}"` strings. A fully- or partially-failed bootstrap looks like success, so geometry can land on missing engineering layers — a documented precondition violation.
- **Fix:** Surface `bootstrap_ok=False` / `status="degraded"` and/or raise `ToolError` when `bootstrap=True` and any linetype/layer reports `failed` or the catch-all fires.
### R19 — ezdxf `dimension_aligned` / `dimension_angular` ship broken (xfail strict=False masks fix/regression)
- **Severity:** MED
- **Location:** markers `tests/test_dimensions.py:16, 23`; root cause `backends/ezdxf_backend.py:696` (`dist=`) and `:711-715` (`center=/p1=/p2=/distance=`)
- **Description:** ezdxf 1.4.3 `add_aligned_dim()` is `(p1, p2, distance, …)` but code passes `dist=`; `add_angular_dim_2l()` is `(base, line1, line2, …)` but code passes `center=/p1=/p2=/distance=` → `TypeError`. Both are advertised MCP tools (`server.py:695, :714`) and recommended in prompt text (`:2639`). `xfail(strict=False)` means an accidental fix XPASSes silently.
- **Fix:** `dist→distance`; `base=(vx,vy)`, `line1=((vx,vy),(x1,y1))`, `line2=((vx,vy),(x2,y2))`. Change markers to `strict=True` (or remove) so the suite alerts.
### R22 — ezdxf transaction snapshots share the single `_undo_stack` with undo history; reload leaves `_current_layer` stale
- **Severity:** MED
- **Location:** `backends/ezdxf_backend.py:213` (single stack), `drawing_undo` `471-485`, `transaction_begin/commit/rollback` `1429-1477`, `system_status` `1489`
- **Description:** One `_undo_stack` is pushed/popped by both undo and transactions, with no txn id/marker/depth isolation; `system_status` reports `len(_undo_stack)` *as* `transaction_depth`. A `drawing_undo` silently consumes an open transaction's snapshot. On reload, undo/rollback re-read the doc but never reset `_current_layer`, so `is_current` reporting goes stale vs the snapshot's `$CLAYER`.
- **Fix:** Use a dedicated transaction list (or begin-markers/ids) distinct from undo history; on reload re-read `$CLAYER` into `self._current_layer`.
### R5 — COM thread/apartment leak on `SendCommand` timeout (no `CoUninitialize`)
- **Severity:** MED
- **Location:** `backends/com_backend.py:416-434` (timeout path), `69-72` (`_com_init`), `387-391` (disconnect)
- **Description:** `_com_init` calls `CoInitialize()` as the executor initializer; `CoUninitialize` appears nowhere in source. On `TimeoutError`, `stuck.shutdown(wait=False)` cannot cancel a worker still blocked in `SendCommand` (the inline comment admits "Abandon it"), and a fresh executor is built — each timeout permanently leaks one `CoInitialize`'d thread + the COM apartment it pinned. `disconnect()` has the same pattern.
- **Fix:** Bound the leak (track abandoned executors, avoid unbounded re-creation); add per-thread `CoUninitialize` via worker-side `try/finally`/thread-local teardown so apartments release when a timed-out thread eventually unblocks.
### R6 — COM backend (1651 lines, 85 async methods) has zero test coverage
- **Severity:** MED
- **Location:** `tests/conftest.py:5` (only `EzdxfBackend` imported); untested logic incl. CMDACTIVE guard `com_backend.py:1474`, transactions `1397-1419`
- **Description:** No test references `ComBackend`/`win32`/`SendCommand`/`CMDACTIVE`. The `backend` fixture instantiates only ezdxf. SendCommand-driven commands, the CMDACTIVE busy-guard + poll loop, and transaction begin/commit/rollback are unexercised, so COM logic can regress silently on non-Windows CI.
- **Fix:** Add `tests/test_com_backend.py` unit-testing the mockable logic (CMDACTIVE guard, `normalize_lineweight` usage, `_safe_send_command` polling, value coercion) with a mocked COM `app`/`doc`, gated by `@pytest.mark.skipif` when `win32com` is unavailable.
### N3 — `entity_select_smart` `length_range` always rejects ARCs (no backend populates ARC `"length"`)
- **Severity:** MED
- **Location:** `backends/base.py:897-904`
- **Description:** The gate fires for `("LINE","ARC")` and reads `e.properties.get("length")`, but the ARC extractor branches (`ezdxf_backend.py:135-139`, `com_backend.py:217-222`) set only center/radius/angles — never `length`. So every ARC has `length is None → return False`, silently excluding all arcs despite the advertised ARC support.
- **Fix:** Populate `props["length"]` for ARC (= `radius * |Δangle|` in radians, with wraparound) in both extractors, OR compute arc length on the fly in `entity_select_smart`, OR drop "ARC" from the gate and docstring.
### N4 — `dimension_auto` hardcodes `layer="DIM"`, which does not exist in the `iso13567` layer set
- **Severity:** MED
- **Location:** `backends/base.py:822, 836, 845, 852`
- **Description:** Every `dimension_linear` inside `dimension_auto` passes `layer="DIM"`, but the iso13567 set's dim layer is `"M-DIMEN-T-N"` (`engineering/layers.py:57`). After `drawing_apply_iso_layers("iso13567")`, dims land on an auto-created `"DIM"` (default color 7 / lw -3), bypassing the ISO layer. `_check_layer_color` won't flag it because `_EXPECTED_LAYER_COLOR` is built only from `ENGINEERING_LAYERS`. (The hardcoded `"CONSTRUCTION"` in `construction_xline/clear`, `base.py:743/:754`, has the same iso13567 mismatch — its construction layer is `"M-CONST-E-N"`.)
- **Fix:** Resolve the dimension (and construction) layer from the active layer set via a registry lookup keyed on `PlanSpec.layer_set_id` (mech→DIM, pid→DIM, iso13567→M-DIMEN-T-N) instead of literals.
### N5 — `EntityInfo.properties` shape diverges by backend (`bounding_box`, TEXT `rotation`, MTEXT `char_height`)
- **Severity:** MED (merges three fresh-sweep property-parity findings)
- **Location:** `bounding_box`: `com_backend.py:192-199` (every entity) vs `ezdxf_backend.py:122` (never); TEXT/MTEXT: `ezdxf_backend.py:144-152` vs `com_backend.py:229-237`.
- **Description:** COM seeds `properties["bounding_box"]` for *every* entity; ezdxf never adds it. ezdxf TEXT sets `rotation` and MTEXT sets `char_height`; COM TEXT/MTEXT set neither (COM MTEXT sets no height at all). Any client reading these keys works on one backend and `KeyError`s on the other.
- **Fix:** Pick a contract and enforce it both ways: add `bounding_box` to ezdxf via `ezdxf.bbox.extents` (or drop it from COM and expose extents only via `analysis_bounding_box`); add `rotation`/`char_height` to the COM TEXT/MTEXT branches (`rad2deg(entity.Rotation)`, `entity.Height`). Document the props contract.
### N6 — COM `system_run_lisp` always reports `result:"nil"` (SendCommand is void)
- **Severity:** MED (duplicates register **R23**'s value-capture half; R23's no-CMDACTIVE-guard half tracked at LOW below)
- **Location:** `backends/com_backend.py:1488-1493`
- **Description:** `result = doc.SendCommand("(progn …)")` then `str(result) if result else "nil"`. AutoCAD COM `SendCommand` is void-returning → `result` is always `None` → the tool unconditionally returns `"nil"` regardless of the actual LISP value.
- **Fix:** Drop the bogus `result` field, or capture out-of-band: `(setvar "USERS1" (vl-princ-to-string <expr>))` then `GetVariable("USERS1")` after completion.
---
## LOW
### R10/R11 residual — _covered under N2 (HIGH)._ No separate LOW entry; the format-default divergence is fully captured there.
### R16 — COM `transaction_commit/rollback` leave `_transaction_active` stale on error; rollback's `UNDO B` bypasses the CMDACTIVE guard
- **Severity:** LOW
- **Location:** `backends/com_backend.py:1406-1423` (commit `1406-1413`, rollback `1415-1423`; begin mirror at `1402-1403`)
- **Description:** `_transaction_active` is set/cleared *after* `await self._run(_sync)` with no `try/finally`; if `_run` raises, the flag stays stale. Rollback's `_sync` issues raw `doc.SendCommand("_UNDO B\n")` (`:1419`) without the CMDACTIVE pre-check/poll that `system_run_command`/`_safe_send_command` use — the documented deadlock hazard.
- **Fix:** Clear `_transaction_active` in a `finally` around `_run`; route rollback's `UNDO B` through `_safe_send_command`.
### R20 — `_registered_tool_count()` reaches through FastMCP private attrs; fails to `-1` and surfaces it
- **Severity:** LOW
- **Location:** `server.py:225-234` (private access at `:231-232`)
- **Description:** `mcp._local_provider._components` + the undocumented `"tool:"` key prefix, with `except Exception: return -1`. `-1` is surfaced unguarded as `tool_count` (`server.py:1969/1976/1983`) and `total_tools` (`:2107`). A minor `fastmcp>=3.0` restructuring silently degrades the count to `-1`.
- **Fix:** Use the public API (e.g. `len(await mcp.get_tools())`), making the helper async.
### R23 — COM `system_run_lisp` has no CMDACTIVE guard (deadlock on prompting LISP)
- **Severity:** LOW (value-capture half merged into N6)
- **Location:** `backends/com_backend.py:1488-1493`
- **Description:** Unlike `system_run_command` (guards at `1473-1482`) and `_safe_send_command` (polls at `1521-1537`), `system_run_lisp` sends raw with no CMDACTIVE pre-check/poll. A prompting form (`(command "_LINE")`, `(getpoint)`) leaves CMDACTIVE non-zero and deadlocks the next COM call.
- **Fix:** Route through `_safe_send_command` (or add the same pre-send guard + post-send poll).
### R24 — ezdxf `_apply_attrs` assigns linetype without `_ensure_linetype_loaded` → undefined names silently render Continuous
- **Severity:** LOW
- **Location:** `backends/ezdxf_backend.py:501-502`
- **Description:** `_apply_attrs` does `entity.dxf.linetype = linetype` with no load check, unlike `entity_set_properties:981`, `layer_create:1030`, `layer_modify:1075`. `entity_create_line` forwards a documented `linetype` param (`server.py:446`; a prompt even says "use entity_create_line with CENTER linetype"), but `_BUILTIN_LINETYPES` is only `{continuous, bylayer, byblock}`, so HIDDEN/CENTER fall through to Continuous.
- **Fix:** Call `_ensure_linetype_loaded(self._require_doc(), linetype)` before the assignment.
### R26 — ezdxf `view_zoom_window` is a no-op but reports success; `view_zoom_and_screenshot` ignores the window region
- **Severity:** LOW
- **Location:** `backends/ezdxf_backend.py:1395-1399` (zoom no-ops), `1401-1425` (screenshot ignores window); `server.py:1887-1893`
- **Description:** ezdxf zoom ops return `{"ok": True, "message": …}` while doing nothing. `view_zoom_and_screenshot` passes window coords to the no-op, then `view_screenshot` renders the entire modelspace via `draw_layout(msp)` with `bbox_inches="tight"`, ignoring `x1/y1/x2/y2` — so the image frames the whole drawing despite the docstring promising a window.
- **Fix:** Store a pending crop region in `view_zoom_window` and apply via `ax.set_xlim/set_ylim` (drop `bbox_inches="tight"` when cropping); or return `ok:False` for ezdxf window zoom so success no longer implies framing.
### R28 — COM screenshot HWND via title-substring `EnumWindows` instead of `AcadApplication.HWND`
- **Severity:** LOW
- **Location:** `backends/com_backend.py:284-301` (`:292`)
- **Description:** `_find_autocad_hwnd` picks the first visible top-level window whose caption contains `"AutoCAD"` — a palette, About dialog, or Start tab can win (EnumWindows ~ top-Z first). The COM app object's `.HWND` (the real main frame) is unused.
- **Fix:** Use `int(_acad_app().HWND)`; keep EnumWindows only as a fallback.
### R21 — Gear flank/root junction is a straight radial segment, not a fillet; radial discontinuity when `root_r >= base_r`
- **Severity:** LOW
- **Location:** `engineering/gear.py:40-50` (flank), `163-190` (root-arc stitch)
- **Description:** `generate_involute_flank` prepends `(root_r, 0.0)` while the involute starts at `(base_r, 0.0)` — both on y=0, so root→base is a straight radial segment, not a trochoid. When `root_r >= base_r` (z≥42 @20°, z≥27 @25°, all within the teeth≥6 API limit) no root point is added but the root arc is drawn at `root_r`, giving a radial jump in the closed polyline (verified m=2,z=60: 1.118 mm). (The teeth≤2 degenerate case is unreachable — tools enforce teeth≥6.)
- **Fix:** Anchor the flank/root junction at `root_r` (radially extend the first involute point, or use a true trochoid below the base circle); skip the root arc when `root_r >= base_r`.
### N7 — `point_from_snap` reads ARC angles via bracket indexing → opaque `KeyError` instead of the function's standard `RuntimeError`
- **Severity:** LOW
- **Location:** `backends/base.py:629-630, 646-647`
- **Description:** ARC `"end"`/`"mid"` branches use `p["start_angle"]`/`p["end_angle"]` direct subscripting, while every other geometry access uses `p.get(...)` + descriptive `RuntimeError`. If a backend's extraction failed (broad try/except swallows at `ezdxf:170`/`com:245`), this raises an opaque `KeyError` rather than the consistent error.
- **Fix:** Use `p.get(...)` with the same None-check `RuntimeError` pattern as `_line_pts`/`_circle_geom`.
### N8 — PlanSpec stored in `_plan_spec` is never read; `drawing_critique` does not "replay" it as documented
- **Severity:** LOW
- **Location:** write `backends/base.py:574`; never read in `engineering/critique.py` `run_critique` (`234-251`); docstrings `base.py:563`, `server.py:2351`
- **Description:** `drawing_plan` stores `self._plan_spec = plan`; docstrings claim it's "held by the backend and replayed by drawing_critique." But `run_critique` and all `_check_*` never reference `_plan_spec`/`.intent`/`.scale`/`.view_count`/`.dim_style` (grep: written once, read nowhere). The committed plan is dead state and the "replay" contract is unmet.
- **Fix:** Implement plan-vs-drawing checks that read `backend._plan_spec` (e.g. view_count vs detected views, dim_style vs dims present), or correct the docstrings to stop claiming replay.
### N9 — `view_zoom_extents`/`view_zoom_window` return different payload shapes (ezdxf adds `message`, COM omits it)
- **Severity:** LOW
- **Location:** `backends/ezdxf_backend.py:1395-1399` vs `backends/com_backend.py:1372-1385`
- **Description:** ezdxf returns `{"ok": True, "message": …}`; COM returns `{"ok": True}` with no `message`. Same MCP tool, two shapes — a client keying on `result["message"]` works on ezdxf and `KeyError`s/None on COM.
- **Fix:** Return identical keys from both backends (always include `message` + an `applied` bool).
---
## SMELL
### R17 — COM `_safe_send_command` polling holds the lone STA thread; inner `deadline_s` decoupled from outer `com_call_timeout`
- **Severity:** SMELL (downgraded from MED — functionally correct; inner 8s deadline is tighter than the 60s outer net, 50 ms sleep is a yielding wait, and holding the STA thread is unavoidable under STA)
- **Location:** `backends/com_backend.py:1498-1542`; `config.py:38`
- **Fix (optional):** Tie `deadline_s = min(8, com_call_timeout*0.8)` so the two stay consistent if the env var changes.
### R29 — Broad `except Exception` guards around optional COM reads
- **Severity:** SMELL (downgraded — adjacent unguarded calls re-raise genuine disconnects into `_run`; most sites log rather than silently pass; reconnect is not bypassed)
- **Location:** `backends/com_backend.py:200, 245, 251, 470, 1447, 1512-1548` (~30 sites)
- **Fix:** Narrow to `except (pywintypes.com_error, AttributeError)` so genuine logic bugs surface.
### S5 — Two parallel, conflicting layer systems
- **Severity:** SMELL
- **Location:** `server.py:1647-1688` (`_LAYER_TEMPLATES`) vs `engineering/layers.py:14-69` (`ENGINEERING_LAYERS`/`LAYER_SET_REGISTRY`)
- **Description:** `_LAYER_TEMPLATES` (architectural/mechanical/…) uses hundredths-int lineweights, all-Continuous linetypes, no CONSTRUCTION layer; `engineering/layers.py` (mech/pid/iso13567) uses mm-floats, real HIDDEN/CENTER, ISO-128 lineweights, CONSTRUCTION. The overlapping mechanical/mech sets *conflict* (HIDDEN/CENTER colors swapped), and `_LAYER_TEMPLATES` violates the ISO-128 discipline `drawing_critique` enforces.
- **Fix:** Retire `_LAYER_TEMPLATES`/`template_apply_layers` in favor of `LAYER_SET_REGISTRY` (or map legacy keys onto it) for one source of truth.
### S3 — `BlockInfo.description` always empty
- **Severity:** SMELL
- **Location:** declared `backends/base.py:71`; never set in `ezdxf_backend.py:1196-1202` / `com_backend.py:1186-1192`
- **Fix:** Populate `description=blk.block.dxf.get("description", "")` (ezdxf) + COM equivalent, or drop the field.
### S1 — `engineering/section.py` helpers (`cutting_plane_line`, `apply_section_hatch`) are dead
- **Severity:** SMELL
- **Location:** `engineering/section.py:12, 65`; only re-exported (`__init__.py:35, 65-66`), never called (`gear_draw_section_aa` builds its hatch inline).
- **Fix:** Wire a `section_*` MCP tool around them, or delete the module + exports.
### S2 — `generate_tooth_profile` is dead (superseded by `generate_full_gear_outline`)
- **Severity:** SMELL
- **Location:** `engineering/gear.py:74`; exported `__init__.py:9, 59`; duplicates ~80% of the outline math (unused local `t_pitch` at `:94`).
- **Fix:** Delete and remove import/`__all__` entries, or wire into a draw path.
### S4 — Stale "Agent B still in flight" build-seam comment + now-vestigial import guards
- **Severity:** SMELL
- **Location:** `engineering/__init__.py:37-39` (comment), `40-51` (try/except guards)
- **Description:** Both `titleblock.py` and `validator.py` now exist and import cleanly, so the guard's rationale is stale. (Related: **R14** — when a guarded symbol *is* None the tools call it and raise an opaque `TypeError`; currently unreachable since modules exist.)
- **Fix:** Delete the build-seam comment; either drop the guards (import directly) or have the tools raise `ToolError("engineering module unavailable: …")` when a symbol is None instead of calling it.
### N10 — `CommandResult` dataclass + `to_dict` never instantiated
- **Severity:** SMELL
- **Location:** `backends/base.py:26-38`; exported `backends/__init__.py:2, 8`
- **Description:** Defined and re-exported, but no constructor call exists outside docs; every command return uses a plain dict. CLAUDE.md/README still list it as a shared dataclass.
- **Fix:** Delete it + its export + doc refs, or actually adopt it as the command return type.
### N11 — `_ACI_NAMES` color-name table defined but never referenced
- **Severity:** SMELL
- **Location:** `backends/ezdxf_backend.py:54-57`
- **Fix:** Delete, or wire into `_entity_info_dxf`/`_layer_info_dxf` as a `color_name` field.
### N12 — `AutoCADBackend.set_layer_active` alias has zero callers
- **Severity:** SMELL
- **Location:** `backends/base.py:910-912`
- **Fix:** Remove; callers use `layer_set_current` directly.
### N13 — `AutoCADBackend.ensure_linetypes` concrete helper is dead (superseded by `engineering.layers.ensure_standard_linetypes`)
- **Severity:** SMELL
- **Location:** `backends/base.py:914-934`
- **Description:** Concrete ABC method with no callers; the real bootstrap path (`server.py:288`, `layers.py:110`) calls the module-level `ensure_standard_linetypes`, which reimplements the same already_loaded/loaded/failed loop.
- **Fix:** Delete it, or refactor `ensure_standard_linetypes` to delegate to it (single owner).
### N14 — COM `self._batch_mode` instance attr written but never read (`_regen` consults only `_COM_STATE`)
- **Severity:** SMELL
- **Location:** `backends/com_backend.py:363, 889, 907, 916, 933` vs `_regen` `180-187`
- **Description:** `_regen` guards on `_COM_STATE.get("batch_mode")` only. The array methods set *both* `self._batch_mode` and `_COM_STATE['batch_mode']`; the instance attr has no readers and can desync.
- **Fix:** Remove the redundant `self._batch_mode` assignments (keep the dict flag), or make `_regen` read the instance attr and drop the dict — one source of truth.
### R27 — Validator step 5 uses substring matching on all TEXT-layer text
- **Severity:** SMELL (downgraded from LOW — gated by `if helix:`, only runs when caller declares `helix_angle`; flagging a SPUR title on a declared-helical part is intended; only the over-broad substring scan is a smell)
- **Location:** `engineering/validator.py:159-181` (`:168`, `:175`)
- **Fix:** Restrict the scan to title-block text and use word-boundary regex (`\bSPUR\b`/`\bHELICAL\b`) instead of `in`.
### R25-residual — `block_find_references` INSERT cap
- **Severity:** SMELL (memory-bug claim REFUTED — `entity_list` defaults to `limit=200`, hard-capped; see appendix)
- **Location:** `server.py:1368-1369`
- **Fix (optional):** Warn when INSERT count hits the 200 cap to avoid silently under-reporting references in large drawings.
---
## (a) Count Summary by Severity
| Severity | Count | IDs |
|---|---|---|
| CRITICAL | 0 | — |
| HIGH | 2 | N1, N2 |
| MED | 13 | R4, R5, R6, R9, R12, R13, R15, R18, R19, R22, N3, N4, N5, N6 *(14 listed — see note)* |
| LOW | 9 | R16, R20, R23, R24, R26, R28, R21, N7, N8, N9 *(10 listed — see note)* |
| SMELL | 13 | R17, R29, R25, R27, S1, S2, S3, S4, S5, N10, N11, N12, N13, N14 *(14 — see note)* |
> Note: counts reflect merges (R10+R11 → N2; R23 split across N6+R23; R13 absorbs two fresh-sweep offset findings; R10 residual folded into N2). Exact kept-finding totals: **HIGH 2, MED 14, LOW 10, SMELL 14 = 40 kept**; 3 dropped (refuted/already-fixed appendix below).
---
## (b) Confirmed P1 Shortlist — highest-leverage fixes to tackle next
1. **N1 — COM hatch associativity/delete bug** (`com_backend.py:659-673`). Flip `AddHatch(…, True)` → `False`. One-line fix, prevents corrupt/invalid hatches in live AutoCAD.
2. **N2 — unify `drawing_save_as` format + derive from extension in `drawing_finalize`** (`base.py:158`, `ezdxf_backend.py:346`, `com_backend.py:516`, `server.py:2299`). Fixes silent DXF-as-DWG mislabeling and cross-backend format divergence on the primary output path.
3. **R19 — fix the two ezdxf dimension calls + make xfail strict** (`ezdxf_backend.py:696, 711-715`; `tests/test_dimensions.py:16,23`). Two advertised tools ship broken; cheap, high-visibility.
4. **R4 — make `dim_overlap` actually run** (`critique.py:199-201` + extractors). Restores a premium-workflow safety check that is currently a guaranteed no-op.
5. **R18 — surface bootstrap failure in `drawing_new`** (`server.py:282-293`). Prevents silent production-precondition violations (geometry on missing layers).
6. **N4 — parameterize `dimension_auto` / construction layers by active layer set** (`base.py:822/836/845/852, 743, 754`). Fixes ISO-13567 dims/construction landing on wrong auto-created layers.
7. **R9 — fix full-circle polar array overlap + COM divergence** (`ezdxf_backend.py:947`). Eliminates duplicate-entity output and a backend behavior split.
8. **R6 — add a mocked COM unit-test suite** (`tests/test_com_backend.py`). Unlocks regression safety for 1651 lines / 85 methods that currently can't be caught on CI — force-multiplier for every other COM fix above.
9. **R5 — bound the COM timeout thread/apartment leak** (`com_backend.py:416-434`). Prevents long-session resource exhaustion in live AutoCAD.
10. **R13/N5 — `entity_offset` side params + property-shape parity** (`ezdxf_backend.py:869-909`, `com_backend.py:861-875`; props extractors). Removes two dead documented params and the cross-backend `properties` contract drift that breaks downstream clients.
---
## (c) Refuted / Already-Fixed Appendix (checked, not actioned)
- **R1 — premium tools missing on COM:** ALREADY FIXED in `cd6329a`. The 8 premium meta-tools are now concrete methods on `AutoCADBackend` (`backends/base.py`); only `_create_xline` is backend-specific.
- **R2 — `int(lineweight)` truncation:** ALREADY FIXED in `cd6329a` via `normalize_lineweight()` in `base.py` (reconciles mm-float vs hundredths-int).
- **R3 — `iso128` critique no-op:** ALREADY FIXED in `cd6329a`.
- **R8 — _(no verification payload supplied; not assessed)._** Listed only for register completeness.
- **R14 — engineering import-guard `TypeError`:** PARTIAL / **not reachable**. Both `titleblock.py` and `validator.py` exist and import cleanly, so the `None`-fallback branch is never taken in the shipped repo. The vestigial guards/comment are carried as **S4** (SMELL).
- **R25 — `block_find_references` unbounded INSERTs + KeyError:** REFUTED. `entity_list` defaults to `limit=200` (hard-capped at all three layers), the opposite of "unbounded"; `e.properties.get("block_name")` cannot `KeyError`. Optional cap-warning carried as a SMELL above.
- **R29 — COM broad `except Exception` masks disconnects:** REFUTED as stated (reconnect not bypassed; most sites log). Retained as SMELL (narrow the guard types).
- **R17 — `_safe_send_command` deadline decoupling as a defect:** the defect framing is REFUTED (inner deadline is the tighter, primary one); retained as SMELL.
- **S4 (`Task #7/#8` phrasing):** the specific cited phrase is REFUTED (the comment says "Agent B … still in flight," not "Task #7/#8"); the substantive build-seam-comment smell STANDS and is kept as S4.