diff --git a/docs/superpowers/plans/2026-09-24-v1.6-understand-implementation.md b/docs/superpowers/plans/2026-09-24-v1.6-understand-implementation.md index e2368c4..2895109 100644 --- a/docs/superpowers/plans/2026-09-24-v1.6-understand-implementation.md +++ b/docs/superpowers/plans/2026-09-24-v1.6-understand-implementation.md @@ -65,7 +65,7 @@ Each line's test lives in the owning task. 1. **AutoCAD text codes in labels** — `%%c51` (AutoCAD's Ø), `\U+2205`, `⌀51`, `ø51` and MTEXT format runs (`{\fArial|b0;Ø51}`, `\P`) must all parse - as the diameter `Ø51`. *Owner: Task 1 (`labels.parse_diameter`).* + as the diameter `Ø51`. *Owner: Task 1 (`labels.parse_diameters`).* 2. **Cyrillic look-alike letters in tags** — a Russian-labelled P&ID types `Т4100` or `М82` with Cyrillic Т/М; it must match the layout's Latin `T4100` / `M82`. *Owner: Task 1 (`labels.parse_tag`).* @@ -3683,7 +3683,7 @@ MSG - Produces: - `engineering/understand/units.py`: `MM_PER_UNIT: dict[str, float]`; `entity_box(rec: EntityRecord) -> tuple[float, float, float, float] | None`; `robust_extents(records: Sequence[EntityRecord]) -> dict` (`computed`, `robust`, `outliers: [{handle, type, layer, at, distance, confidence}]` farthest first, `stretch`, `method`); `wall_segments(records, *, is_wall=None) -> tuple[list[tuple[Pt, Pt]], int]`; `infer_units(snap: Snapshot, *, records=None, room_areas=()) -> dict` (`declared: {code, name, label}`, `inferred: str | None`, `mm_per_unit: float | None`, `agree: bool`, `confidence`, `scores`, `evidence`, `warning: str | None`). - `engineering/understand/clusters.py`: `find_clusters(records, *, exclude=(), cell=None) -> tuple[list[dict], dict[str, str]]` — rows `{id ("C1", "C2", … largest first), box, size, entities, layers, labels}` and handle → cluster id. - - `engineering/understand/describe.py`: `describe(snap: Snapshot) -> dict` with the skeleton's keys `units`, `extents`, `clusters`, `layers`, `equipment_tags`, `rooms`, `languages`, `blocks`, `layouts`, `title_block`, `warnings` (plus `source`, `entities`, `room_faces`); `prepare(snap: Snapshot) -> dict` (`model`, `extents`, `kept`, `clusters`, `membership`, `diag`); `tag_occurrences(snap: Snapshot, *, prep: dict | None = None) -> list[dict]` (`tag`, `handle`, `at`, `space`, `layer`, `cluster`, `source`, `confidence`); `find_rooms(records, *, tol: float, membership: dict) -> dict` (`rooms`, `labelled: [(row, Face)]`, `faces`, `faces_unlabelled`, `wall_segments`, `arcs_skipped`, `skipped`); `text_language(text: str) -> str`; `FACE_TOL_FRACTION`. Task 4 imports `prepare`, `tag_occurrences`, `find_rooms` and `FACE_TOL_FRACTION`. The takeoffs (Task 6, group N) do not import them: group N is cut from `main` beside this branch, so `takeoff.py` carries its own narrow `unit_of` / `room_regions` (Task 6, "Why this module carries stand-ins"). + - `engineering/understand/describe.py`: `describe(snap: Snapshot) -> dict` with the skeleton's keys `units`, `extents`, `clusters`, `layers`, `equipment_tags`, `rooms`, `languages`, `blocks`, `layouts`, `title_block`, `warnings` (plus `source`, `entities`, `room_faces`); `prepare(snap: Snapshot) -> dict` (`model`, `extents`, `kept`, `clusters`, `membership`, `diag`); `tag_occurrences(snap: Snapshot, *, prep: dict | None = None) -> list[dict]` (`tag`, `handle`, `at`, `space`, `layer`, `cluster`, `source`, `confidence`); `find_rooms(records, *, tol: float, membership: dict) -> dict` (`rooms`, `labelled: [(row, Face)]`, `faces`, `faces_unlabelled`, `wall_segments`, `arcs_skipped`, `skipped`); `text_language(text: str) -> str`; `FACE_TOL_FRACTION`. Task 4 imports `prepare`, `tag_occurrences`, `find_rooms` and `FACE_TOL_FRACTION`. The takeoffs (Task 6, group N) do not import them: group N is cut from `main` beside this branch, so `takeoff.py` carries its own narrow `unit_of` / `room_regions` (Task 6, "Why this module carries stand-ins") until Task 11 (Steps 15-20) re-points them at these readers. - MCP tool `drawing_understand(path: str | None = None) -> dict` (core pack, read-only, tags `analysis` + `query`). - [ ] **Step 1: Write the failing units test** @@ -5738,7 +5738,7 @@ MSG - Produces: - `engineering/understand/scale.py`: `match_tags(a: Snapshot, b: Snapshot, *, cluster_a=None, cluster_b=None) -> dict` — `{"matched": {tag: (pa, pb)}, "ambiguous": [tag, …], "occurrences": {tag: {"a": n, "b": m}}, "only_a": [...], "only_b": [...], "cluster_a", "cluster_b"}`; a cluster id is one of `find_clusters`' `"C1"`, `"C2"`, …, None is the whole model space, and an unknown id is refused with the ids the drawing has. - `scale_check(a: Snapshot, b: Snapshot, *, min_pair_mm=2000.0) -> dict` — `{"verdict": "to_scale"|"schematic"|"partly"|"insufficient", "factor", "median", "q1", "q3", "iqr", "within_10pct", "pair_count", "pairs", "pairs_truncated", "pairs_below_floor", "matched": [{tag, a, b}], "ambiguous", "occurrences", "only_a", "only_b", "groups": [{group, kind, tags, verdict, factor, median, q1, q3, iqr, within_10pct, pair_count}], "scope": {"cluster_a", "cluster_b"}, "units": {"a", "b"}, "min_pair_mm", "thresholds", "notes"}`; `b` is the reference. - - Constants `WITHIN = 0.10`, `TO_SCALE_SHARE = 0.80`, `SCHEMATIC_SHARE = 0.50`, `MIN_TAGS = 3` (spec §6). Task 6 (group N, cut beside this branch) cannot import this module: `pipe_rows(..., scale=...)` accepts a `scale_check(pid, layout)` result in this shape (Task 12's benchmark, smoke and field-test script pass one) and otherwise computes its own `scale_stand_in`; its layout tag positions come from its own `layout_tags`, which applies the same rule — tags unique within the chosen cluster only, the rest `ambiguous` — so Review Focus 3 is reused as a rule, not as an import. + - Constants `WITHIN = 0.10`, `TO_SCALE_SHARE = 0.80`, `SCHEMATIC_SHARE = 0.50`, `MIN_TAGS = 3` (spec §6). Task 6 (group N, cut beside this branch) cannot import this module: `pipe_rows(..., scale=...)` accepts a `scale_check(pid, layout)` result in this shape (Task 12's benchmark, smoke and field-test script pass one) and otherwise computes its own `scale_stand_in` until Task 11 (Steps 15-20) replaces it with this function; its layout tag positions come from its own `layout_tags`, which applies the same rule — tags unique within the chosen cluster only, the rest `ambiguous` — so Review Focus 3 is reused as a rule, not as an import. - MCP tool `drawing_scale_check(reference: str, path: str | None = None, min_pair_mm: float = 2000.0) -> dict` (core pack, read-only, tags `analysis` + `query`). - [ ] **Step 1: Write the failing scale test** @@ -7915,7 +7915,7 @@ MSG - Consumes: Task 5 — `build_network(...) -> dict` (its `runs`, `attachments` and `stats.tol` / `stats.label_search` / `stats.overlap_length`), `Run`, `tag_occurrences(snap)`, `MODEL`, `TEXT_TYPES`; Task 1 — `Pt`, `Snapshot`, `read_snapshot`, `vocab.SERVICES`, `vocab.classify_layer`, `vocab.room_label(text) -> dict | None`; track F — `engineering.arch.faces.planar_faces`, `face_containing`, `Face`, `DEFAULT_TOL` (reused, not re-implemented); Task 2 — `build_plant_pair` truth keys `pid`, `layout`, `scale_verdict`, `layout_insunits`, `layout_true_unit`, `pipe_runs`, `rooms`. A `scale` argument in the shape of group U's `scale_check` result (`verdict`, `factor`, `median`, `iqr`, `within_10pct`, `pairs`, `matched`, `ambiguous`, `groups`) is consumed when a caller passes one. - Produces: in `engineering/understand/route.py` — `manhattan(a: Pt, b: Pt) -> float`, `rmst(points: Sequence[Pt]) -> tuple[float, tuple[tuple[int, int], ...]]`, `route_legs(a: Pt, b: Pt) -> tuple[tuple[Pt, Pt], ...]`. In `engineering/understand/takeoff.py` — `pipe_rows(network, pid: Snapshot, layout: Snapshot | None, *, scale=None, services=DEFAULT_SERVICES, allowance=0.20, vertical_allowance=0.0, length_source="auto", force=False) -> dict` (keys `kind`, `rows`, `runs`, `control`, `summary{by_service, by_diameter, by_room}`, `totals{routed_m, unroutable_m, net_m, allowance, allowance_m, total_m, flagged_runs}`, `length_source`, `scale`, `scale_verified`, `units{pid, layout}`, `warnings`, `method`); `DEFAULT_SERVICES = ("product", "cip_supply", "cip_return")`, `LENGTH_SOURCES`, `UNIT_METRES`; `check_pipe_options(*, services, allowance, vertical_allowance, length_source) -> dict`; `check_allowance(allowance) -> float`; `unit_of(snap) -> dict` (`declared`, `inferred`, `m_per_unit`, `warning`, `evidence`); `layout_tags(snap, wanted=()) -> dict` (`positions`, `ambiguous`, `cluster`, `clusters`); `scale_stand_in(pid, layout, *, pid_tags=None, layout_found=None, layout_unit=None) -> dict` (the subset of group U's `scale_check` shape a takeoff reads: `verdict`, `factor` (median, only when `to_scale`), `median`, `q1`, `q3`, `iqr`, `within_10pct`, `pair_count`, `pairs[{tags, a_mm, b_mm, ratio}]`, `matched[{tag, a, b}]`, `ambiguous`, `groups: []`, `notes`); `room_regions(snap, cluster=None) -> dict` (`faces: [(key, Face)]`, `labels: [(key, Pt)]`); `room_at(regions, p) -> str`; `apportion(legs, regions) -> dict[str, float]`. -**Why this module carries stand-ins.** Group N is cut from `main` beside groups U and D (skeleton, Execution), so `engineering/understand/{units,clusters,scale}.py` do not exist on this branch. `takeoff.py` therefore carries the narrow form of each that a takeoff needs — `unit_of`, `layout_tags` with its plan-copy clusters, and `scale_stand_in` (spec §6's statistics without per-room groups) — and `pipe_rows` uses a `scale` result it is given before its own. Tag positions on the layout always come from `layout_tags`, which applies Review Focus item 3's rule (only tags unique inside the chosen cluster; the rest `ambiguous`, never picked). The stand-in returns group U's result shape — the ratio is P&ID millimetres over layout millimetres, each drawing in its inferred unit, and `factor` is set only when the verdict is `to_scale` — so after the merge a caller can pass `scale_check(pid, layout)` into `pipe_rows(scale=...)` without touching a row — Task 12's benchmark task, correctness checks, smoke and field-test script do exactly that. No task of this plan swaps `unit_of` / `room_regions` for group U's `infer_units` / `find_rooms`; the stand-ins stay in `takeoff.py`, and the `pipe_takeoff` tool keeps computing its own `scale_stand_in`. The unit bands (text 0.5 mm – 1 m, span 0.1 m – 5 km) and the 2 % cluster gap are this module's heuristics, stated in its docstring — not standards values. Review Focus item 5 is owned here: `test_review_focus_5_inches_declared_over_millimetres` (hand-built) and `test_review_focus_5_the_synthetic_layout_is_read_in_millimetres` (the Task 2 pair). +**Why this module carries stand-ins.** Group N is cut from `main` beside groups U and D (skeleton, Execution), so `engineering/understand/{units,clusters,scale}.py` do not exist on this branch. `takeoff.py` therefore carries the narrow form of each that a takeoff needs — `unit_of`, `layout_tags` with its plan-copy clusters, and `scale_stand_in` (spec §6's statistics without per-room groups) — and `pipe_rows` uses a `scale` result it is given before its own. Tag positions on the layout always come from `layout_tags`, which applies Review Focus item 3's rule (only tags unique inside the chosen cluster; the rest `ambiguous`, never picked). The stand-in returns group U's result shape — the ratio is P&ID millimetres over layout millimetres, each drawing in its inferred unit, and `factor` is set only when the verdict is `to_scale` — so after the merge a caller can pass `scale_check(pid, layout)` into `pipe_rows(scale=...)` without touching a row — Task 12's benchmark task, correctness checks, smoke and field-test script do exactly that. The stand-ins are debt with a due date: on main, Task 11 (Steps 15-20) re-points `unit_of`, `layout_tags` and `room_regions` at group U's readers and deletes `scale_stand_in`, so the four tools read one unit, one plan copy, one verdict and one set of rooms per drawing. Until then they must follow spec §5 / §6 closely enough that the swap changes no behavioural assertion of this task. The unit bands (text 0.5 mm – 1 m, span 0.1 m – 5 km) and the 2 % cluster gap are this module's heuristics, stated in its docstring — not standards values. Review Focus item 5 is owned here: `test_review_focus_5_inches_declared_over_millimetres` (hand-built) and `test_review_focus_5_the_synthetic_layout_is_read_in_millimetres` (the Task 2 pair). - [ ] **Step 1: Write the failing route test** @@ -15362,11 +15362,14 @@ MSG - Modify: `tests/test_tool_registry.py` (frozen group-size snapshot: 247 tools, 28 groups) - Modify: `docs/tool-inventory.json` (regenerated, never hand-edited) - Modify: `README.md` (release snapshot tools / advertised / collected tests, the alias-corpus sentence, the Discovery row, the `` pack line, the `TOOL_PROFILE` and `TOOL_PACKS` configuration rows) -- Test: `tests/test_tool_packs.py`, `tests/test_discovery_aliases.py`, `tests/test_discovery_transform.py`, `tests/test_tool_registry.py`, `tests/test_token_suite.py` +- Modify: `engineering/understand/describe.py` (`drawing_units`), `engineering/understand/scale.py` (units through `drawing_units`), `engineering/understand/takeoff.py` (the stand-ins give way to group U's readers) +- Create: `tests/test_understand_one_reading.py` +- Modify: `tests/test_understand_takeoff.py`, `tests/test_understand_report.py` (the assertions that pinned the stand-ins) +- Test: `tests/test_tool_packs.py`, `tests/test_discovery_aliases.py`, `tests/test_discovery_transform.py`, `tests/test_tool_registry.py`, `tests/test_token_suite.py`, `tests/test_understand_one_reading.py` **Interfaces:** -- Consumes: the six tools the groups registered — SECTION 26 `drawing_understand`, `drawing_scale_check` (group U, Tasks 3-4, tagged `{"analysis", "query"}`), `drawing_diff`, `drawing_topology_check` (group D, Tasks 9-10, tagged `{"analysis", "query"}`); SECTION 27 `pipe_takeoff`, `cable_takeoff` (group N, Tasks 7-8, tagged `{"plant", "analysis"}`) — each with its alias record in `discovery/aliases.py` (every one `acad=()` except `drawing_diff`'s `("COMPARE",)`), and each branch's paragraph in the `test_tool_group_sizes_are_unchanged` docstring saying the six filed under `analysis` while `understand` / `plant` were unranked; `server._registered_tools()`, `server._tool_groups()`, `server._apply_tool_profile(profile) -> {"disabled_tools", "enabled_count", "tool_packs"}`; `discovery.aliases.TOOL_ALIASES`, `ToolAliases(acad=(), synonyms=())`, `SHARED_ACAD_COMMANDS`. -- Produces: `server.TOOL_PACK_NAMES == ("core", "pid", "settings", "mech", "arch", "plant")`, `server.PACK_TOOL_NAMES["plant"] == frozenset({"pipe_takeoff", "cable_takeoff"})`, `len(server.LEAN_TOOL_NAMES) == 65` (55 with `TOOL_PACKS=core`), groups `understanding` (the four SECTION 26 tools, core pack) and `plant` (exactly `PACK_TOOL_NAMES["plant"]`) — 247 tools in 28 groups; `discovery/aliases.py::_TRACK_H_VOCABULARY` and `_fold_in(records, extra)`; eight golden tuning cases. +- Consumes: the six tools the groups registered — SECTION 26 `drawing_understand`, `drawing_scale_check` (group U, Tasks 3-4, tagged `{"analysis", "query"}`), `drawing_diff`, `drawing_topology_check` (group D, Tasks 9-10, tagged `{"analysis", "query"}`); SECTION 27 `pipe_takeoff`, `cable_takeoff` (group N, Tasks 7-8, tagged `{"plant", "analysis"}`) — each with its alias record in `discovery/aliases.py` (every one `acad=()` except `drawing_diff`'s `("COMPARE",)`), and each branch's paragraph in the `test_tool_group_sizes_are_unchanged` docstring saying the six filed under `analysis` while `understand` / `plant` were unranked; `server._registered_tools()`, `server._tool_groups()`, `server._apply_tool_profile(profile) -> {"disabled_tools", "enabled_count", "tool_packs"}`; `discovery.aliases.TOOL_ALIASES`, `ToolAliases(acad=(), synonyms=())`, `SHARED_ACAD_COMMANDS`. For the one-reading steps (15-20): Task 3 — `describe.prepare(snap) -> dict` (`kept`, `clusters: [{id, box, ...}]`, `membership`, `diag`), `find_rooms(records, *, tol, membership) -> dict` (`rooms`, `labelled: [(row, Face)]`), `FACE_TOL_FRACTION`, `describe(snap)` (`units`, `rooms`), `units.infer_units(snap, *, records=None, room_areas=()) -> dict` (`declared: {code, name, label}`, `inferred`, `mm_per_unit`, `evidence`, `warning`); Task 4 — `scale.scale_check(a, b, *, min_pair_mm=2000.0) -> dict` (`units: {a, b: {unit, mm_per_unit, declared}}`, `groups`), `scale._side(snap)`; Task 6 — `takeoff.unit_of`, `layout_tags`, `scale_stand_in`, `room_regions`, `pipe_rows`, `UNIT_METRES`; Task 8 — `takeoff.cable_rows`. +- Produces: `server.TOOL_PACK_NAMES == ("core", "pid", "settings", "mech", "arch", "plant")`, `server.PACK_TOOL_NAMES["plant"] == frozenset({"pipe_takeoff", "cable_takeoff"})`, `len(server.LEAN_TOOL_NAMES) == 65` (55 with `TOOL_PACKS=core`), groups `understanding` (the four SECTION 26 tools, core pack) and `plant` (exactly `PACK_TOOL_NAMES["plant"]`) — 247 tools in 28 groups; `discovery/aliases.py::_TRACK_H_VOCABULARY` and `_fold_in(records, extra)`; eight golden tuning cases. From the one-reading steps: `describe.drawing_units(snap, *, prep=None, rooms=None) -> dict` (an `infer_units` result, the one call `describe`, `scale_check` and the takeoffs make); `takeoff.unit_of(snap, *, prep=None)`, `layout_tags(snap, wanted=(), *, prep=None)` and `room_regions(snap, cluster=None, *, prep=None)` with their result shapes unchanged; `takeoff.scale_stand_in` and the takeoff's own unit bands and cluster grid removed. - [ ] **Step 1: Write the failing test for the new surface — replace `tests/test_tool_packs.py`** @@ -16253,7 +16256,418 @@ If the printed dict differs anywhere but by a tool that lacks its section tag (S Run: `uv run --frozen pytest tests/test_tool_registry.py tests/test_tool_inventory.py tests/test_release_consistency.py::test_section_header_counts_match_decorators -q` Expected: all pass. -- [ ] **Step 15: Retake the README figures the tree derives** +- [ ] **Step 15: One reading — write the failing test** + +Groups U and N were cut from one base side by side, so Task 6's `takeoff.py` carries narrow stand-ins for what group U reads properly: `unit_of` (its own text-height and span bands), a 2 % grid of plan copies inside `layout_tags`, `scale_stand_in` (spec §6 without per-room groups) and `room_regions`. On main both readers exist, and two readers of one drawing can disagree: on the hand-built layout of Task 6 (INSUNITS inches over millimetre geometry, no door, no wall pair), `infer_units` called without room faces scores mm, cm and inches 0.75 each and the header's tie-break answers inches, while the takeoff's bands answer mm. From here on every reader asks one function, `describe.drawing_units` (the call `describe` already makes: kept records, labelled room faces as evidence), and the takeoffs read plan copies, scale and rooms through group U's modules. The stand-ins go. + +Create `tests/test_understand_one_reading.py`: + +```python +"""One reading of a drawing: the takeoffs see what drawing_understand and drawing_scale_check see. + +Groups U and N were built side by side from one base, so Task 6 carried narrow +stand-ins: its own unit bands, a 2 % grid of plan copies, a scale statistic +without room groups, its own room faces. On main the takeoffs read units, plan +copies, scale and rooms through group U's readers, so the four tools never +give one drawing two units, two plan copies, two verdicts or two sets of rooms. +""" + +from __future__ import annotations + +import pytest + +from engineering.understand import takeoff +from engineering.understand.describe import describe, drawing_units, prepare +from engineering.understand.network import build_network +from engineering.understand.scale import scale_check +from engineering.understand.snapshot import Snapshot, read_snapshot +from tests.fixtures.plant_pair import build_plant_pair + + +@pytest.fixture(scope="module") +def plant(tmp_path_factory): + truth = build_plant_pair(tmp_path_factory.mktemp("one_reading")) + return truth, read_snapshot(truth["pid"]), read_snapshot(truth["layout"]) + + +def empty(insunits): + return Snapshot( + source="empty.dxf", + insunits=insunits, + extmin=None, + extmax=None, + layers={}, + layouts=(), + records=(), + ) + + +def test_one_unit_per_drawing(plant): + _truth, pid, layout = plant + check = scale_check(pid, layout) + for snap, side in ((pid, "a"), (layout, "b")): + unit = takeoff.unit_of(snap) + assert unit["inferred"] == describe(snap)["units"]["inferred"] + assert unit["inferred"] == check["units"][side]["unit"] + assert takeoff.unit_of(layout)["inferred"] == "mm" # INSUNITS says inches: Review Focus 5 + + +def test_the_takeoff_unit_carries_drawing_units_evidence_and_warning(plant): + _truth, pid, layout = plant + for snap in (pid, layout): + mine, theirs = takeoff.unit_of(snap), drawing_units(snap) + assert mine["declared"] == theirs["declared"]["name"] + assert mine["m_per_unit"] == pytest.approx(theirs["mm_per_unit"] / 1000.0) + assert mine["evidence"] == theirs["evidence"] + expected = f"{snap.source}: {theirs['warning']}" if theirs["warning"] else None + assert mine["warning"] == expected + + +def test_a_unit_nothing_measures_keeps_the_declared_one(): + unit = takeoff.unit_of(empty(5)) + assert (unit["declared"], unit["inferred"], unit["m_per_unit"], unit["warning"]) == ( + "cm", + "cm", + 0.01, + None, + ) + + +def test_a_unitless_drawing_nothing_measures_is_read_in_mm_and_says_so(): + unit = takeoff.unit_of(empty(0)) + assert (unit["declared"], unit["inferred"], unit["m_per_unit"]) == (None, "mm", 0.001) + assert unit["warning"] == ( + "empty.dxf: INSUNITS declares Unitless (0) and no measurement settles the unit; " + "the takeoff reads it as mm." + ) + + +def test_plan_copies_are_the_clusters_drawing_understand_reports(plant): + truth, _pid, layout = plant + found = takeoff.layout_tags(layout, truth["tag_positions"]["layout"]) + boxes = [tuple(cluster["box"]) for cluster in prepare(layout)["clusters"]] + assert found["clusters"] == len(boxes) == truth["cluster_count"] + assert tuple(found["cluster"]) in boxes + + +def test_the_takeoff_scale_is_drawing_scale_check(plant): + _truth, pid, layout = plant + result = takeoff.pipe_rows(build_network(pid), pid, layout) + check = scale_check(pid, layout) + for key in ("verdict", "factor", "median", "within_10pct", "pair_count", "groups"): + assert result["scale"][key] == check[key], key + + +def test_rooms_are_the_rooms_drawing_understand_reports(plant): + truth, _pid, layout = plant + cluster = takeoff.layout_tags(layout, truth["tag_positions"]["layout"])["cluster"] + regions = takeoff.room_regions(layout, cluster) + assert sorted(key for key, _face in regions["faces"]) == sorted( + str(room["number"]) for room in truth["rooms"] + ) + reported = { + (str(row["number"] or row["name"]), round(row["face"]["area"], 3)) + for row in describe(layout)["rooms"] + if row["face"] is not None + } + for key, face in regions["faces"]: + assert (key, round(face.area, 3)) in reported + + +def test_no_second_reader_is_left(): + for name in ( + "scale_stand_in", + "INSUNITS_NAMES", + "TEXT_BAND_M", + "SPAN_BAND_M", + "TYPICAL_SPAN_M", + "TYPICAL_TEXT_M", + "CLUSTER_GAP_SHARE", + "MIN_PAIR_MM", + "MIN_TAGS", + "SCALE_BAND", + "TO_SCALE_SHARE", + "SCHEMATIC_SHARE", + "_model_points", + "_robust_box", + "_text_heights", + "_mark_segment", + "_clusters", + ): + assert not hasattr(takeoff, name), name +``` + +- [ ] **Step 16: Run it to verify it fails** + +Run: `uv run --frozen pytest tests/test_understand_one_reading.py -q` +Expected: FAIL — the import of `drawing_units` fails first (`ImportError: cannot import name 'drawing_units'`). + +- [ ] **Step 17: `describe.drawing_units` — the one call every reader makes** + +In `engineering/understand/describe.py`, add `"drawing_units"` to `__all__` (after `"describe"`) and add this function directly after `find_rooms`: + +```python +def drawing_units(snap: Snapshot, *, prep: dict | None = None, rooms: dict | None = None) -> dict: + """`units.infer_units` the one way every reader calls it: on the records kept + after the outliers, with the labelled room faces as evidence. + + `describe`, `scale.scale_check` and the takeoffs all read a drawing's unit + here, so no two tools report two units for one drawing. ``prep`` and + ``rooms`` are a `prepare` / `find_rooms` result the caller already holds. + """ + prep = prep or prepare(snap) + if rooms is None: + tol = prep["diag"] * FACE_TOL_FRACTION if prep["diag"] > 0 else 1.0 + rooms = find_rooms(prep["kept"], tol=tol, membership=prep["membership"]) + return infer_units( + snap, + records=prep["kept"], + room_areas=[row["face"]["area"] for row, _face in rooms["labelled"]], + ) +``` + +In `describe()`, replace + +```python + units = infer_units( + snap, + records=prep["kept"], + room_areas=[row["face"]["area"] for row, _face in rooms["labelled"]], + ) +``` + +with + +```python + units = drawing_units(snap, prep=prep, rooms=rooms) +``` + +In `engineering/understand/scale.py`, import `drawing_units` from `engineering.understand.describe` beside `FACE_TOL_FRACTION, find_rooms, prepare, tag_occurrences`, drop the now unused `from engineering.understand.units import infer_units`, and in `scale_check`'s units loop replace + +```python + areas = [row["face"]["area"] for row, _f in rooms["labelled"]] if rooms else [] + u = infer_units(snap, records=side["kept"], room_areas=areas) +``` + +with + +```python + u = drawing_units(snap, prep=side, rooms=rooms) +``` + +(`side` is `_side(snap)`: `prepare`'s dict plus `tags`, so it carries `kept`, `diag` and `membership`. Drawing `a` now contributes its own labelled rooms as unit evidence, as `describe` always did; on a P&ID without room faces nothing changes.) + +- [ ] **Step 18: The takeoffs read through group U** + +In `engineering/understand/takeoff.py`: + +1. Imports — add + +```python +from engineering.understand.describe import FACE_TOL_FRACTION, drawing_units, find_rooms, prepare +from engineering.understand.scale import scale_check +``` + + and let `uv run --frozen ruff check engineering/understand/takeoff.py` name what became unused (`statistics`, `DEFAULT_TOL`, `planar_faces` are the likely ones); remove exactly what it names. + +2. Delete the stand-ins and everything only they read: `INSUNITS_NAMES`, `TEXT_BAND_M`, `SPAN_BAND_M`, `TYPICAL_SPAN_M`, `TYPICAL_TEXT_M`, `CLUSTER_GAP_SHARE`, `MIN_PAIR_MM`, `MIN_TAGS`, `SCALE_BAND`, `TO_SCALE_SHARE`, `SCHEMATIC_SHARE` (each with its `#:` comment), `_model_points`, `_robust_box`, `_text_heights`, `_mark_segment`, `_clusters` and `scale_stand_in`; remove `"scale_stand_in"` from `__all__`. `UNIT_METRES` stays: the takeoff still converts with it. + +3. Replace `unit_of` with: + +```python +def unit_of(snap: Snapshot, *, prep: dict | None = None) -> dict: + """The unit a drawing's geometry implies - `describe.drawing_units`, the reading + `drawing_understand` and `drawing_scale_check` give - in the shape a takeoff reads. + + ``{"declared", "inferred", "m_per_unit", "warning", "evidence"}``: ``declared`` + is the INSUNITS abbreviation (None when it names none of mm / cm / m / in / + ft), ``evidence`` is `infer_units`' own list. When no measurement settles + the unit the declared one stands, else mm, and the warning says which; a + takeoff never leaves a length in unknown units. + """ + found = drawing_units(snap, prep=prep) + name = found["declared"]["name"] + declared = name if name in UNIT_METRES else None + inferred, warning = found["inferred"], found["warning"] + if inferred is None: + inferred = declared or "mm" + warning = ( + f"INSUNITS declares {found['declared']['label']} ({found['declared']['code']}) and " + f"no measurement settles the unit; the takeoff reads it as {inferred}." + ) + return { + "declared": declared, + "inferred": inferred, + "m_per_unit": UNIT_METRES[inferred], + "warning": f"{snap.source}: {warning}" if warning else None, + "evidence": found["evidence"], + } +``` + +4. In `layout_tags`, give the signature a `prep` keyword — `def layout_tags(snap: Snapshot, wanted: Iterable[str] = (), *, prep: dict | None = None) -> dict:` — replace `_gap, boxes = _clusters(snap)` with + +```python + prep = prep or prepare(snap) + boxes = [tuple(cluster["box"]) for cluster in prep["clusters"]] +``` + + and in its docstring replace "Otherwise the drawing is split into clusters (plan copies, sheets, a far outlier) and the cluster holding the most `wanted` tags is chosen" with "Otherwise the drawing's clusters - `describe.prepare`'s, the plan copies, sheets and outliers `drawing_understand` reports - are read, and the cluster holding the most `wanted` tags is chosen". The rest of the function is unchanged; `cluster` stays a box. + +5. Replace `room_regions` with: + +```python +def room_regions(snap: Snapshot, cluster=None, *, prep: dict | None = None) -> dict: + """Room faces and labels - `describe.find_rooms`, the rooms `drawing_understand` + reports - inside ``cluster`` (a box) when one is given. + + ``{"faces": [(key, Face)], "labels": [(key, Pt)]}``; the key is the room + number, else its name. A record belongs to the cluster when every point + it has lies in the box. + """ + prep = prep or prepare(snap) + + def keep(rec) -> bool: + return cluster is None or all( + cluster[0] <= p[0] <= cluster[2] and cluster[1] <= p[1] <= cluster[3] + for p in rec.points + ) + + tol = prep["diag"] * FACE_TOL_FRACTION if prep["diag"] > 0 else 1.0 + found = find_rooms( + [rec for rec in prep["kept"] if keep(rec)], tol=tol, membership=prep["membership"] + ) + + def key(row: dict) -> str: + return str(row["number"] or row["name"] or row["text"]) + + return { + "faces": [(key(row), face) for row, face in found["labelled"]], + "labels": sorted((key(row), (row["at"][0], row["at"][1])) for row in found["rooms"]), + } +``` + +6. In `pipe_rows`, read each drawing's first pass once and take the scale from `scale_check`. Replace + +```python + pid_unit = unit_of(pid) + layout_unit = unit_of(layout) if layout is not None else None + wanted = {tag for run in network["runs"] for tag in run.tags} + found = layout_tags(layout, wanted) if layout is not None else None + if scale is None: + scale = ( + scale_stand_in(pid, layout, layout_found=found, layout_unit=layout_unit) + if layout is not None + else {"verdict": "not_checked", "pair_count": 0, "matched": [], "ambiguous": []} + ) +``` + + with + +```python + prep_pid = prepare(pid) + prep_layout = prepare(layout) if layout is not None else None + pid_unit = unit_of(pid, prep=prep_pid) + layout_unit = unit_of(layout, prep=prep_layout) if layout is not None else None + wanted = {tag for run in network["runs"] for tag in run.tags} + found = layout_tags(layout, wanted, prep=prep_layout) if layout is not None else None + if scale is None: + scale = ( + scale_check(pid, layout) + if layout is not None + else {"verdict": "not_checked", "pair_count": 0, "matched": [], "ambiguous": []} + ) +``` + + and + +```python + regions = ( + room_regions(layout, found["cluster"]) + if found is not None and source == "layout" + else room_regions(pid) + ) +``` + + with + +```python + regions = ( + room_regions(layout, found["cluster"], prep=prep_layout) + if found is not None and source == "layout" + else room_regions(pid, prep=prep_pid) + ) +``` + + In its docstring, "`scale` is a scale-check result (`scale_check`'s shape); omitted, the stand-in runs when a layout is given." becomes "`scale` is a `scale_check` result; omitted, `scale_check(pid, layout)` runs when a layout is given." + +7. In `cable_rows`, replace `layout_unit = unit_of(layout) if layout is not None else None` with + +```python + prep_layout = prepare(layout) if layout is not None else None + layout_unit = unit_of(layout, prep=prep_layout) if layout is not None else None +``` + + `placed = layout_tags(layout, set(loads) | set(pid_callouts))` with `placed = layout_tags(layout, set(loads) | set(pid_callouts), prep=prep_layout)`, and `regions = room_regions(layout, cluster) if layout is not None else room_regions(pid)` with `regions = room_regions(layout, cluster, prep=prep_layout) if layout is not None else room_regions(pid)`. + +8. In the module docstring, replace the paragraph that begins "This module also carries the takeoff's own stand-ins" with: + +``` +Units, plan copies, scale and rooms are read through group U's readers +(`describe.drawing_units`, `describe.prepare`, `scale.scale_check`, +`describe.find_rooms`), so a takeoff sees exactly what `drawing_understand` +and `drawing_scale_check` report for the same drawing. +``` + +- [ ] **Step 19: Re-point the tests that pinned the stand-ins** + +In `tests/test_understand_takeoff.py`: + +- add `from engineering.understand.scale import scale_check` between the `network` and `snapshot` imports, and delete `scale_stand_in,` from the `engineering.understand.takeoff` import list; +- rename `test_the_stand_in_recognises_a_uniform_scale_and_a_rearranged_one` to `test_scale_check_recognises_a_uniform_scale_and_a_rearranged_one` and replace its two calls `scale_stand_in(half, layout)` and `scale_stand_in(other, layout)` with `scale_check(half, layout)` and `scale_check(other, layout)` — every assertion in it stays; +- the unit warning now carries `infer_units`' wording (`INSUNITS declares Inches (1) but the geometry reads as mm: ...`, prefixed by the drawing's source), so `"INSUNITS declares in" in w and "as mm" in w` becomes `"INSUNITS declares Inches (1)" in w and "reads as mm" in w` in `test_review_focus_5_inches_declared_over_millimetres`, and `"INSUNITS declares in" in w` becomes `"INSUNITS declares Inches (1)" in w` in `test_review_focus_5_the_synthetic_layout_is_read_in_millimetres`. + +In `tests/test_understand_report.py`, `test_the_method_sheet_is_generated_from_the_result`: `assert any("INSUNITS declares in" in w for w in warnings)` becomes `assert any("INSUNITS declares Inches (1)" in w for w in warnings)`. Its `method["Scale check: pairs"] == "12"` stays: that test passes its own `SCHEMATIC` result, and a given `scale` is still used as given. + +No other assertion changes. If a behavioural assertion of groups U or N fails now — a unit, a length, a status, a room, a verdict — the reading is wrong, not the test: fix the reader against spec §5 / §6 and say which in the report; never weaken the assertion. + +- [ ] **Step 20: Gates and commit** + +``` +uv run --frozen ruff format engineering/understand tests/test_understand_one_reading.py tests/test_understand_takeoff.py tests/test_understand_report.py +uv run --frozen ruff check engineering tests +uv run --frozen pytest tests/test_understand_one_reading.py tests/test_understand_units.py tests/test_understand_describe.py tests/test_understand_scale.py tests/test_understand_scale_tools.py tests/test_understand_takeoff.py tests/test_understand_report.py tests/test_understand_cable.py tests/test_understand_tools.py -q +uv run --frozen pytest -q --deselect tests/test_release_consistency.py::test_document_numbers_match_the_tree +``` + +Expected: ruff clean; `tests/test_understand_one_reading.py` 8 passed; the whole suite green with the collected-tests figure deselected (this step adds eight tests; Step 21 retakes the figure). + +``` +git add engineering/understand/describe.py engineering/understand/scale.py engineering/understand/takeoff.py tests/test_understand_one_reading.py tests/test_understand_takeoff.py tests/test_understand_report.py +git -c user.name="Umut CAN" -c user.email="umutcan.edizaslan@gmail.com" commit -m "$(cat <<'MSG' +refactor(understand): one reading of units, plan copies, scale and rooms + +Groups U and N were built side by side, so the takeoffs carried narrow +stand-ins for what group U reads properly. On main they read through U: + +- describe.drawing_units is the one call every reader makes (kept records, + labelled room faces as evidence); describe, scale_check and the takeoffs + all use it. Without the room faces, a layout that declares inches over + millimetres can tie three ways and fall to the header. +- layout_tags takes its plan copies from describe.prepare, room_regions its + rooms from describe.find_rooms, and pipe_rows its scale from scale_check, + per-room groups included. scale_stand_in and the takeoff's own unit bands + and cluster grid are gone. +- test_understand_one_reading pins the agreement: one unit, one plan copy, + one verdict and one set of rooms per drawing, across drawing_understand, + drawing_scale_check and the two takeoffs. + +Co-Authored-By: Claude Opus 5.5 (1M context) +Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd +MSG +)" +``` + +- [ ] **Step 21: Retake the README figures the tree derives** ``` uv run --frozen python scripts/check_doc_numbers.py --list @@ -16286,7 +16700,7 @@ Then: Run: `uv run --frozen python scripts/check_doc_numbers.py` Expected: exit 0. This task changes no benchmark figure — `correctness_suite.py` still holds the 43 checks the published A/B report was run over — so the only numbers that moved are the ones just rewritten. A `DRIFT` line names the file and the figure; fix that figure, never the checker. (The README token table is remeasured by Task 12.) -- [ ] **Step 16: Gates** +- [ ] **Step 22: Gates** ``` uv run --frozen ruff format server.py discovery/aliases.py tests/test_tool_packs.py tests/test_tool_registry.py tests/test_token_suite.py tests/test_discovery_aliases.py tests/data/discovery_golden.py @@ -16295,9 +16709,9 @@ uv run --frozen python scripts/generate_tool_inventory.py --check uv run --frozen pytest -q ``` -Expected: ruff clean; no inventory drift; the whole suite green, `tests/test_release_consistency.py` in full included — the collected-tests figure was retaken in Step 15, so the test every earlier task deselected passes again on main. +Expected: ruff clean; no inventory drift; the whole suite green, `tests/test_release_consistency.py` in full included — the collected-tests figure was retaken in Step 21, so the test every earlier task deselected passes again on main. -- [ ] **Step 17: Commit** +- [ ] **Step 23: Commit** ``` git add server.py discovery/aliases.py docs/tool-inventory.json README.md tests/test_tool_packs.py tests/test_tool_registry.py tests/test_token_suite.py tests/test_discovery_aliases.py tests/data/discovery_golden.py