fix(pid): pid_from_spec refuses non-finite numbers by path and rolls back on any interruption

_num let NaN/inf (and a 400-digit int, as OverflowError) through, so an
INSERT was placed at [nan, 0] and the refusal, when it came, named the
line instead of equipment[0].x. run_spec caught Exception only, so a
CancelledError mid-run left the placed symbols inside an open transaction
(an open undo mark on COM). Finite check in _num; except BaseException.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd

engineering/pid/spec.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import copy
+import math
 from typing import TYPE_CHECKING
 
 from .critique import PID_FOCUSES, issues_for
@@ -135,9 +136,24 @@ _LINE_KEYS = {
 
 
 def _num(value, where: str) -> float:
+    """A finite number, or a ``ValueError`` naming the path.
+
+    ``float("nan")`` used to pass and be placed as an INSERT at ``[nan, 0]``;
+    the transaction committed and ``build_graph`` then crashed on the drawing
+    with no path in its message. Only in-process callers can deliver a
+    non-finite (the MCP pipeline coerces them first), but ``run_spec`` is a
+    public interface and its docstring promises nothing malformed reaches
+    the drawing.
+    """
     if isinstance(value, bool) or not isinstance(value, (int, float)):
         raise ValueError(f"{where} must be a number")
-    return float(value)
+    try:
+        number = float(value)
+    except OverflowError as exc:  # a 400-digit int is a number but not a float
+        raise ValueError(f"{where} must be a finite number") from exc
+    if not math.isfinite(number):
+        raise ValueError(f"{where} must be a finite number")
+    return number
 
 
 def _ref(value, where: str, ids: set[str]) -> tuple[str, str | None]:
@@ -422,7 +438,11 @@ async def run_spec(backend: AutoCADBackend, spec: dict, dry_run: bool = False) -
             except ValueError as exc:
                 raise ValueError(f"{where}: {exc}") from exc
             crossings_total += drawn["crossings"]
-    except Exception:
+    except BaseException:
+        # ``Exception`` alone let a client cancellation (``CancelledError`` is
+        # a ``BaseException``) leave the placed symbols in the drawing inside
+        # an open transaction — on COM with the undo mark still open, so every
+        # later ``pid_from_spec`` was refused until a manual rollback.
         await backend.transaction_rollback()
         raise
     await backend.transaction_commit()

server.py

@@ -6837,11 +6837,13 @@ async def pid_from_spec(
 ) -> dict:
     """Equipment, valves, instruments and connectors placed, then every line
     drawn port-to-port, inside one transaction: any bad item rolls the whole
-    sheet back and the error names it (`lines[2].to`). The response carries the
+    sheet back and the error names it (`lines[2].to`); so does an interruption
-    graph read back from the drawing and the P&ID critique issues, so the
+    (a client cancellation mid-run leaves no half-drawn sheet and no open
-    caller sees dangling ends or duplicate tags in the same round trip.
+    transaction). A non-finite coordinate, rotation, scale or stub is refused
-    `dry_run` returns the planned vertices and crossings without touching the
+    by path before anything is placed. The response carries the graph read
-    drawing.
+    back from the drawing and the P&ID critique issues, so the caller sees
+    dangling ends or duplicate tags in the same round trip. `dry_run` returns
+    the planned vertices and crossings without touching the drawing.
     """
     from engineering.pid.spec import run_spec
 

tests/test_pid_from_spec.py

@@ -85,3 +85,45 @@ async def test_finalize_scores_the_example_at_ninety_or_better(client, tmp_path)
     ).structured_content
     assert final["score"]["score"] >= 90, final["score"]
     assert not [i for i in final.get("critique", []) if str(i.get("focus", "")).startswith("pid_")]
+
+
+# ── Track A hardening (track E wave 0, Task 5) ───────────────────────────────
+
+
+@pytest.mark.parametrize(
+    "mutate, fragment",
+    [
+        (lambda s: s["equipment"][0].update({"x": float("nan")}), "equipment[0].x"),
+        (lambda s: s["valves"][0].update({"y": float("inf")}), "valves[0].y"),
+        (lambda s: s["instruments"][0].update({"rotation": 10**400}), "instruments[0].rotation"),
+        (lambda s: s["lines"][0].update({"stub": float("-inf")}), "lines[0].stub"),
+    ],
+)
+async def test_non_finite_numbers_are_refused_by_path_before_any_write(backend, mutate, fragment):
+    before = await backend.entity_count()
+    spec = copy.deepcopy(EXAMPLE_SPEC)
+    mutate(spec)
+    with pytest.raises(ValueError, match="finite") as exc:
+        await run_spec(backend, spec)
+    assert fragment in str(exc.value)
+    assert await backend.entity_count() == before
+    assert (await backend.system_status())["transaction_depth"] == 0
+
+
+async def test_a_cancellation_mid_run_rolls_the_sheet_back(backend, monkeypatch):
+    """``CancelledError`` is a ``BaseException``; ``except Exception`` let the
+    placed symbols stay inside an open transaction."""
+    import asyncio
+
+    from engineering.pid import spec as spec_module
+
+    before = await backend.entity_count()
+
+    async def cancelled(*args, **kwargs):
+        raise asyncio.CancelledError()
+
+    monkeypatch.setattr(spec_module, "draw_line", cancelled)
+    with pytest.raises(asyncio.CancelledError):
+        await run_spec(backend, EXAMPLE_SPEC)
+    assert await backend.entity_count() == before, "the placed symbols were rolled back"
+    assert (await backend.system_status())["transaction_depth"] == 0, "no open transaction"