docs: Track A (P&ID) implementation plan — 21 TDD tasks; spec reconciled
Plan covers the prerequisite block_insert attribute fix (ezdxf wrapped the INSERT in an anonymous *U block and lost its ATTRIBs), the two generic contracts, the 43-symbol catalogue, routing, the ISA-5.1 grammar, the graph reader, deliverables, six critique focuses, pid_from_spec, TOOL_PACKS, resources, benchmark v4, and the live COM smoke. Spec reconciled with the plan: 43 base symbols, FUNC/LOOP at +-2.0, solid = DXF SOLID entity, shape/direction options, example spec uses an actuated FCV-101 (a hand valve has no signal port), refine hints in detail["hint"], preflight LAYER_SET_INTENT_MISMATCH, radial port matching on the circle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
co-authored by
Claude Opus 5
parent
3430573de1
commit
40eb37cf6f
File diff suppressed because it is too large
Load Diff
@@ -1,7 +1,7 @@
|
|||||||
# v1.6 Track A — P&ID expert and reader
|
# v1.6 Track A — P&ID expert and reader
|
||||||
|
|
||||||
**Date:** 2026-09-15 · **Status:** design approved in conversation, awaiting
|
**Date:** 2026-09-15 · **Status:** approved; implementation plan at
|
||||||
written review · **Parent:** `2026-09-15-v1.6-roadmap-design.md` (track A) ·
|
`docs/superpowers/plans/2026-09-15-v1.6-pid-implementation.md` · **Parent:** `2026-09-15-v1.6-roadmap-design.md` (track A) ·
|
||||||
**Baseline:** v1.5.1
|
**Baseline:** v1.5.1
|
||||||
|
|
||||||
## 1. Summary
|
## 1. Summary
|
||||||
@@ -95,9 +95,9 @@ get the whole track for the price of the two new contracts.
|
|||||||
Rotation 0 = flow left→right (process `in` port on −X, `out` on +X).
|
Rotation 0 = flow left→right (process `in` port on −X, `out` on +X).
|
||||||
- All primitives inside the block are on layer `"0"` with colour/linetype/
|
- All primitives inside the block are on layer `"0"` with colour/linetype/
|
||||||
lineweight **ByBlock**, so the INSERT's layer (section 4.6) supplies the ISO
|
lineweight **ByBlock**, so the INSERT's layer (section 4.6) supplies the ISO
|
||||||
128 lineweight. Filled shapes (check-valve flap, flow arrow) are closed
|
128 lineweight. Filled shapes (flow arrow, blind side of a spectacle blind) are
|
||||||
polylines plus a SOLID-pattern hatch spec; `block_define` supports
|
DXF SOLID entities; `block_define` supports `{"type": "solid", "points": [...]}`
|
||||||
`{"type": "solid", "points": [...]}` for exactly this.
|
for exactly this.
|
||||||
|
|
||||||
### 4.3 Record structure
|
### 4.3 Record structure
|
||||||
|
|
||||||
@@ -131,7 +131,7 @@ from the drawing, not the catalogue, so old drawings keep working.
|
|||||||
|
|
||||||
### 4.4 Catalogue v1
|
### 4.4 Catalogue v1
|
||||||
|
|
||||||
Base symbols (41) — every one is a test case:
|
Base symbols (43: 34 equipment/valve bodies, the instrument generator, 3 connectors, 5 markers) — every one is a test case:
|
||||||
|
|
||||||
| Family | Symbols | Ports |
|
| Family | Symbols | Ports |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
@@ -178,7 +178,7 @@ apply the whole set.
|
|||||||
|---|---|---|
|
|---|---|---|
|
||||||
| valve, rotating, vessel, heat, misc, connector | `TAG` (height 2.5, centred at `(0, ymax + 3.5)`); `DESC` (height 2.0, centred at `(0, ymax + 6.5)`, visible; `pid_symbol_insert` writes `""` when no description is given — an empty ATTRIB renders nothing, and per-insert invisibility is not something `block_insert(attributes=…)` exposes) | above the body |
|
| valve, rotating, vessel, heat, misc, connector | `TAG` (height 2.5, centred at `(0, ymax + 3.5)`); `DESC` (height 2.0, centred at `(0, ymax + 6.5)`, visible; `pid_symbol_insert` writes `""` when no description is given — an empty ATTRIB renders nothing, and per-insert invisibility is not something `block_insert(attributes=…)` exposes) | above the body |
|
||||||
| valve with actuator | + `FAIL` (height 2.0, right of the actuator) | |
|
| valve with actuator | + `FAIL` (height 2.0, right of the actuator) | |
|
||||||
| instrument | `FUNC` (height 2.0, middle-centre at `(0, +1.25)`), `LOOP` (height 2.0, middle-centre at `(0, −3.25)`) | the two halves of the bubble |
|
| instrument | `FUNC` (height 2.0, middle-centre at `(0, +2.0)`), `LOOP` (height 2.0, middle-centre at `(0, −2.0)`) | the two halves of the bubble |
|
||||||
| offpage | `TAG` inside the arrow box; `LINK` invisible | |
|
| offpage | `TAG` inside the arrow box; `LINK` invisible | |
|
||||||
|
|
||||||
Attribute values are what `block_get_attributes` returns and what the reader
|
Attribute values are what `block_get_attributes` returns and what the reader
|
||||||
@@ -212,7 +212,7 @@ Entity specs (all coordinates block-local; `layer` optional, default `"0"`;
|
|||||||
| `arc` | `cx, cy, r, start_deg, end_deg` (CCW, r > 0) |
|
| `arc` | `cx, cy, r, start_deg, end_deg` (CCW, r > 0) |
|
||||||
| `polyline` | `points: [[x, y], ...]` (≥ 2), `closed: bool`, optional `bulges: [float, ...]` same length as `points` |
|
| `polyline` | `points: [[x, y], ...]` (≥ 2), `closed: bool`, optional `bulges: [float, ...]` same length as `points` |
|
||||||
| `text` | `text, x, y, height` (> 0), optional `rotation_deg` (0), `align` ∈ {`left`, `center`, `right`, `middle_center`} (`left`) |
|
| `text` | `text, x, y, height` (> 0), optional `rotation_deg` (0), `align` ∈ {`left`, `center`, `right`, `middle_center`} (`left`) |
|
||||||
| `solid` | `points` (3 or 4) — written as a closed polyline plus a SOLID hatch |
|
| `solid` | `points` (3 or 4, polygon order) — written as a DXF SOLID entity (`add_solid` / `AddSolid`); a quad's third and fourth points are swapped into SOLID's bow-tie vertex order |
|
||||||
|
|
||||||
ATTDEF specs: `tag` (`^[A-Z][A-Z0-9_]{0,30}$`, unique within the block),
|
ATTDEF specs: `tag` (`^[A-Z][A-Z0-9_]{0,30}$`, unique within the block),
|
||||||
`prompt` (default = tag), `default` (default `""`), `x, y, height`, optional
|
`prompt` (default = tag), `default` (default `""`), `x, y, height`, optional
|
||||||
@@ -311,7 +311,9 @@ pid_symbol_insert(symbol: str, x: float, y: float, rotation: float = 0.0, scale:
|
|||||||
tag: str | None = None, desc: str | None = None,
|
tag: str | None = None, desc: str | None = None,
|
||||||
actuator: str | None = None, fail: str | None = None,
|
actuator: str | None = None, fail: str | None = None,
|
||||||
type: str | None = None, location: str | None = None, # instruments
|
type: str | None = None, location: str | None = None, # instruments
|
||||||
params: dict | None = None, # parametric
|
params: dict | None = None, # parametric vessels
|
||||||
|
shape: str | None = None, # reducer: concentric | eccentric
|
||||||
|
direction: str | None = None, # offpage: in | out
|
||||||
link: str | None = None, # offpage
|
link: str | None = None, # offpage
|
||||||
layer: str | None = None)
|
layer: str | None = None)
|
||||||
→ {"handle", "block_name", "defined": bool, "layers_created": [...],
|
→ {"handle", "block_name", "defined": bool, "layers_created": [...],
|
||||||
@@ -552,8 +554,10 @@ Offpage {link, nodes: [...]} # connectors sharing a LIN
|
|||||||
`INSTRUMENT-LINE-SIGNAL → electric` when the effective linetype is dashed,
|
`INSTRUMENT-LINE-SIGNAL → electric` when the effective linetype is dashed,
|
||||||
otherwise `signal_unknown` (`pneumatic` is never assumed from a plain line);
|
otherwise `signal_unknown` (`pneumatic` is never assumed from a plain line);
|
||||||
any other layer → `null`.
|
any other layer → `null`.
|
||||||
5. **Resolve endpoints** through a uniform grid (cell = 10·tolerance): a port
|
5. **Resolve endpoints** through a uniform grid (cell = 10·tolerance): a
|
||||||
within `tolerance` → `{node, port}`; else another edge's vertex within
|
bubble's radial port matches a line end *on its circle*
|
||||||
|
(`|dist(end, centre) − r| ≤ tolerance`); any other port within `tolerance`
|
||||||
|
→ `{node, port}`; else another edge's vertex within
|
||||||
`tolerance` → junction; else a point within `tolerance` of another edge's
|
`tolerance` → junction; else a point within `tolerance` of another edge's
|
||||||
segment interior → junction (the touched edge is not split); else
|
segment interior → junction (the touched edge is not split); else
|
||||||
`dangling` with the nearest port within `10·tolerance` as a hint.
|
`dangling` with the nearest port within `10·tolerance` as a hint.
|
||||||
@@ -610,11 +614,11 @@ the path, e.g. `lines[2].from`):
|
|||||||
{"id": "V-201", "symbol": "vertical_vessel", "x": 220, "y": 120,
|
{"id": "V-201", "symbol": "vertical_vessel", "x": 220, "y": 120,
|
||||||
"params": {"width": 20, "height": 40, "nozzles": [{"name": "N1", "side": "top", "fraction": 0.5},
|
"params": {"width": 20, "height": 40, "nozzles": [{"name": "N1", "side": "top", "fraction": 0.5},
|
||||||
{"name": "N2", "side": "left", "fraction": 0.3}]}}],
|
{"name": "N2", "side": "left", "fraction": 0.3}]}}],
|
||||||
"valves": [{"id": "HV-101", "symbol": "gate", "actuator": "hand", "x": 160, "y": 104, "tag": "HV-101"}],
|
"valves": [{"id": "FCV-101", "symbol": "globe", "actuator": "diaphragm", "x": 160, "y": 104, "tag": "FCV-101", "fail": "FC"}],
|
||||||
"instruments": [{"id": "FIC-101", "type": "dcs", "location": "primary", "x": 160, "y": 140, "tag": "FIC-101"}],
|
"instruments": [{"id": "FIC-101", "type": "dcs", "location": "primary", "x": 160, "y": 140, "tag": "FIC-101"}],
|
||||||
"lines": [{"from": "P-101.discharge", "to": "HV-101.in", "class": "process_major", "number": "100-P-1001-CS1"},
|
"lines": [{"from": "P-101.discharge", "to": "FCV-101.in", "class": "process_major", "number": "100-P-1001-CS1"},
|
||||||
{"from": "HV-101.out", "to": "V-201.N2", "class": "process_major", "number": "100-P-1001-CS1"},
|
{"from": "FCV-101.out", "to": "V-201.N2", "class": "process_major", "number": "100-P-1001-CS1"},
|
||||||
{"from": "FIC-101", "to": "HV-101.signal", "class": "electric"}],
|
{"from": "FIC-101", "to": "FCV-101.signal", "class": "electric"}],
|
||||||
"connectors": [{"id": "OP-1", "direction": "out", "x": 300, "y": 120, "tag": "TO P&ID-002", "link": "L-17",
|
"connectors": [{"id": "OP-1", "direction": "out", "x": 300, "y": 120, "tag": "TO P&ID-002", "link": "L-17",
|
||||||
"from": "V-201.N1", "class": "process_major"}]}
|
"from": "V-201.N1", "class": "process_major"}]}
|
||||||
```
|
```
|
||||||
@@ -651,20 +655,25 @@ within the run) and return `[]` when `stats["nodes_by_kind"]` is empty, so a
|
|||||||
drawing without P&ID content is never scored on these rules. They run under
|
drawing without P&ID content is never scored on these rules. They run under
|
||||||
`focus=None` and therefore inside `drawing_finalize`.
|
`focus=None` and therefore inside `drawing_finalize`.
|
||||||
|
|
||||||
### 11.2 Refine hints (`engineering/refiner.py` registry)
|
### 11.2 Refine hints
|
||||||
|
|
||||||
No deterministic repair moves geometry. `pid_dangling_line` → hint naming the
|
No deterministic repair moves geometry, so no P&ID focus registers a repair
|
||||||
nearest port and the `pid_line_draw` call that would connect it;
|
handler: `drawing_refine` reports them as `manual_required` and carries each
|
||||||
`pid_duplicate_tag` / `pid_illegal_tag` / `pid_untagged_instrument` → hint
|
issue through unchanged. The hint rides in the issue itself, `detail["hint"]`:
|
||||||
naming the node and `block_set_attributes`; `pid_incompatible_connection` →
|
`pid_dangling_line` → the nearest port and the `pid_line_draw` call that would
|
||||||
hint to redraw with the right class; `pid_unconnected_equipment` → informational.
|
connect it; `pid_duplicate_tag` / `pid_illegal_tag` / `pid_untagged_instrument`
|
||||||
|
→ the node and `block_set_attributes`; `pid_incompatible_connection` → redraw
|
||||||
|
with the right class; `pid_unconnected_equipment` → informational.
|
||||||
|
|
||||||
### 11.3 Plan
|
### 11.3 Plan and preflight
|
||||||
|
|
||||||
`detect_ambiguities` gains one rule: intent text matches
|
One rule, in `engineering/preflight.py`: intent text matches
|
||||||
`\b(p&id|pid|piping and instrumentation|process flow|instrument)\b` (case-
|
`\b(p&id|pid|piping and instrumentation|process flow|instrument)\b` (case-
|
||||||
insensitive) while `layer_set_id != "pid"` → clarification "intent reads as a
|
insensitive) while `layer_set_id != "pid"`. `preflight_drawing` reports it as
|
||||||
P&ID but the layer set is `<id>`; pass `layer_set_id='pid'`".
|
the conflict `LAYER_SET_INTENT_MISMATCH` (so the preflight is not `ready`), and
|
||||||
|
`drawing_plan` — which can be called without a preflight — returns the same
|
||||||
|
sentence under a `warnings` key: "intent reads as a P&ID but the layer set is
|
||||||
|
`<id>`; pass `layer_set_id='pid'`".
|
||||||
|
|
||||||
### 11.4 Prompt
|
### 11.4 Prompt
|
||||||
|
|
||||||
@@ -725,6 +734,11 @@ Both are static, cost nothing in the tool budget, and are added to
|
|||||||
|
|
||||||
Unit (headless, CI):
|
Unit (headless, CI):
|
||||||
|
|
||||||
|
- `tests/test_block_insert_attributes.py` — the prerequisite fix: v1.5.1's
|
||||||
|
headless `block_insert(attributes=…)` used `add_auto_blockref`, which wraps
|
||||||
|
the INSERT in an anonymous `*U` block, so the returned handle names `*U1`
|
||||||
|
and `block_get_attributes` finds nothing (reproduced 2026-09-15). The fix
|
||||||
|
attaches ATTRIBs to the INSERT itself; the test pins block name and values.
|
||||||
- `tests/test_pid_symbols.py` — for **every** catalogue entry and every
|
- `tests/test_pid_symbols.py` — for **every** catalogue entry and every
|
||||||
generated variant: primitives pass `block_define` validation, declared
|
generated variant: primitives pass `block_define` validation, declared
|
||||||
`bbox` contains every primitive, every port lies on the bbox boundary (or at
|
`bbox` contains every primitive, every port lies on the bbox boundary (or at
|
||||||
@@ -760,6 +774,10 @@ Unit (headless, CI):
|
|||||||
- `tests/test_pid_from_spec.py` — the section 10 example end to end; a bad
|
- `tests/test_pid_from_spec.py` — the section 10 example end to end; a bad
|
||||||
reference rolls back (entity count unchanged); `dry_run` writes nothing;
|
reference rolls back (entity count unchanged); `dry_run` writes nothing;
|
||||||
`drawing_finalize` score ≥ 90.
|
`drawing_finalize` score ≥ 90.
|
||||||
|
- `tests/test_pid_insert.py`, `tests/test_pid_line_draw.py`,
|
||||||
|
`tests/test_pid_resources_prompt.py`, `tests/test_benchmark_v4.py` — the
|
||||||
|
insertion service, the line tool end to end, the two resources and the
|
||||||
|
rewritten prompt, and the v4 matrix.
|
||||||
- `tests/test_tool_packs.py` — `TOOL_PACKS=core` hides every `pid_*` tool and
|
- `tests/test_tool_packs.py` — `TOOL_PACKS=core` hides every `pid_*` tool and
|
||||||
keeps `block_define`; unknown pack ignored with `ignored`; `core` cannot be
|
keeps `block_define`; unknown pack ignored with `ignored`; `core` cannot be
|
||||||
disabled; interaction with `lean` and `ENABLE_3D`.
|
disabled; interaction with `lean` and `ENABLE_3D`.
|
||||||
|
|||||||
Reference in New Issue
Block a user