From c4759e696d839e20538ac779fcd7abab50505579 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Thu, 24 Sep 2026 12:51:10 +0300 Subject: [PATCH] fix(understand): the pipe network's tolerance follows the P&ID's unit, and a dropped stub is said pipe_takeoff merged line ends closer than tol = 1.0 drawing units and silently dropped any segment shorter than that. On a millimetre P&ID that is 1 mm. On a metre drawing it is a metre: ends a metre apart were merged and every pipe under a metre disappeared from the totals, with no warning. The review of the network engine found this. - tol now defaults to 1 mm in the unit the P&ID's geometry implies: 1.0 on a millimetre drawing, 0.001 on a metre one. A tol the caller gives is used as given. - build_network counts the segments that collapse to a point (stats.segments_below_tol), and pipe_rows turns a non-zero count into a warning naming the tolerance. A new wire test builds a metre P&ID with a 0.6 m pipe and a 5 m pipe. Before this change the takeoff said 5.0 m; now it says 5.6 m. The network test checks that a sub-tolerance stub is counted. Registry, packs, aliases and every track H suite: 472 passed. The inventory matches the live surface. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/understand/draw.py | 10 +++++++--- engineering/understand/network.py | 4 ++++ engineering/understand/takeoff.py | 7 +++++++ server.py | 10 ++++++---- tests/test_understand_network.py | 8 ++++++++ tests/test_understand_takeoff_tools.py | 19 +++++++++++++++++++ 6 files changed, 51 insertions(+), 7 deletions(-) diff --git a/engineering/understand/draw.py b/engineering/understand/draw.py index c1f3dbd..0ba308e 100644 --- a/engineering/understand/draw.py +++ b/engineering/understand/draw.py @@ -21,6 +21,7 @@ from engineering.understand.takeoff import ( check_pipe_options, check_section_rules, pipe_rows, + unit_of, ) __all__ = ["run_cable_takeoff", "run_pipe_takeoff"] @@ -49,7 +50,7 @@ async def run_pipe_takeoff( length_source: str = "auto", force: bool = False, layers=None, - tol: float = 1.0, + tol: float | None = None, ) -> dict: """`pipe_rows` over a P&ID and an optional layout, plus the workbook when `output` is named. @@ -64,14 +65,17 @@ async def run_pipe_takeoff( vertical_allowance=vertical_allowance, length_source=length_source, ) - tol = _check_tol(tol) + tol = None if tol is None else _check_tol(tol) if length_source == "layout" and not layout_path: raise ValueError("length_source='layout' needs a layout drawing; none was given") pid = await take_snapshot(backend, pid_path) layout = await take_snapshot(backend, layout_path) if layout_path else None def compute() -> dict: - network = build_network(pid, layers=layers, tol=tol) + # 1 mm in the unit the P&ID's geometry implies: a fixed 1.0 would be a + # metre on a metre drawing, merging ends a metre apart and dropping pipes + joint = tol if tol is not None else 1.0 / (unit_of(pid)["m_per_unit"] * 1000.0) + network = build_network(pid, layers=layers, tol=joint) return pipe_rows( network, pid, diff --git a/engineering/understand/network.py b/engineering/understand/network.py index 459d19f..6529519 100644 --- a/engineering/understand/network.py +++ b/engineering/understand/network.py @@ -539,10 +539,13 @@ def build_network(snap: Snapshot, *, layers=None, tol=1.0, label_search=None) -> raw = _segments(snap, chosen) verts = _Vertices(tol) snapped = [] + below_tol = 0 # segments whose two ends merge into one vertex: dropped, and counted for a, b, bulge, handle, layer in raw: u, v = verts.index(a), verts.index(b) if u != v: snapped.append((u, v, bulge, handle, layer)) + else: + below_tol += 1 # equipment first: an INSERT a tag labels is never a fitting occurrences = tag_occurrences(snap) @@ -888,6 +891,7 @@ def build_network(snap: Snapshot, *, layers=None, tol=1.0, label_search=None) -> "tol": tol, "label_search": search, "segments": len(raw), + "segments_below_tol": below_tol, "pieces": len(pieces), "overlap_removed": overlap_count, "overlap_length": overlap_length, diff --git a/engineering/understand/takeoff.py b/engineering/understand/takeoff.py index 5d35b57..ed679cd 100644 --- a/engineering/understand/takeoff.py +++ b/engineering/understand/takeoff.py @@ -802,6 +802,13 @@ def pipe_rows( ) } warnings = [u["warning"] for u in (pid_unit, layout_unit) if u and u["warning"]] + dropped = int(network.get("stats", {}).get("segments_below_tol") or 0) + if dropped: + warnings.append( + f"{dropped} P&ID segment(s) shorter than the junction tolerance " + f"({network['stats']['tol']:g} drawing units) collapsed to a point and are not " + "counted" + ) if not scale_verified: warnings.append( "scale_verified: false - lengths are drawn lengths on a P&ID that no scale check " diff --git a/server.py b/server.py index 0d4ebf4..994695c 100644 --- a/server.py +++ b/server.py @@ -11124,13 +11124,15 @@ async def pipe_takeoff( ), ] = None, tol: Annotated[ - float, + float | None, Field( - default=1.0, + default=None, gt=0.0, - description="Line ends closer than this (P&ID drawing units) are one junction.", + description="Line ends closer than this (P&ID drawing units) are one junction; " + "default 1 mm in the unit the P&ID's geometry implies (1.0 on a millimetre " + "drawing, 0.001 on a metre one).", ), - ] = 1.0, + ] = None, ctx: Context = None, ) -> dict: """A pipe takeoff: topology from the P&ID, lengths from the layout, rows by room x service x diameter. diff --git a/tests/test_understand_network.py b/tests/test_understand_network.py index 282d576..3ccf2d2 100644 --- a/tests/test_understand_network.py +++ b/tests/test_understand_network.py @@ -103,6 +103,14 @@ def test_ends_within_tol_merge_into_one_run(): assert run.ends == ((0.0, 0.0), (2000.0, 0.0)) +def test_a_segment_shorter_than_tol_is_counted_when_it_is_dropped(): + # a 0.5-unit stub collapses to one vertex at tol 1.0: gone from the network, + # but never silently - stats say how many went + result = net(line("1", (0.0, 0.0), (1000.0, 0.0)), line("2", (2000.0, 0.0), (2000.5, 0.0))) + assert result["stats"]["segments_below_tol"] == 1 + assert net(line("1", (0.0, 0.0), (1000.0, 0.0)))["stats"]["segments_below_tol"] == 0 + + def test_a_branch_ending_on_a_header_is_a_t_junction(): result = net(line("1", (0.0, 0.0), (2000.0, 0.0)), line("2", (1000.0, 0.0), (1000.0, 800.0))) (run,) = result["runs"] diff --git a/tests/test_understand_takeoff_tools.py b/tests/test_understand_takeoff_tools.py index db15d8e..070142a 100644 --- a/tests/test_understand_takeoff_tools.py +++ b/tests/test_understand_takeoff_tools.py @@ -37,6 +37,25 @@ def plant(tmp_path): return build_plant_pair(tmp_path) +async def test_a_metre_pid_keeps_a_pipe_shorter_than_one_drawing_unit(client, tmp_path): + # The junction tolerance defaults to 1 mm in the unit the P&ID's geometry + # implies. A fixed 1.0 would be a metre here: the 0.6 m pipe would collapse + # to a point and vanish from the totals. + import ezdxf + + doc = ezdxf.new(setup=False) + doc.header["$INSUNITS"] = 6 # metres + msp = doc.modelspace() + msp.add_line((0.0, 0.0), (0.6, 0.0), dxfattribs={"layer": "PRODUCT"}) + msp.add_line((10.0, 0.0), (15.0, 0.0), dxfattribs={"layer": "PRODUCT"}) + msp.add_text("NOTES", height=0.25, dxfattribs={"layer": "TEXT"}).set_placement((0.0, 1.0)) + path = tmp_path / "metre_pid.dxf" + doc.saveas(path) + result = (await client.call_tool("pipe_takeoff", {"pid_path": str(path)})).structured_content + assert result["units"]["pid"]["inferred"] == "m" + assert result["totals"]["net_m"] == pytest.approx(5.6) # 0.6 + 5.0 + + async def test_pipe_takeoff_measures_on_the_layout_and_writes_the_workbook(client, plant, tmp_path): output = tmp_path / "takeoff.xlsx" arguments = {