From a2996b0c9578120cd9de22ab8cd468a35cfe0b73 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Wed, 23 Sep 2026 22:48:36 +0300 Subject: [PATCH] fix(task-5): skip what the room reader cannot read, and one face carries one room read_segments trusted any row with `points`. On a live seat an old-style AcDb2dPolyline's Coordinates are x, y, z triples (measured on AutoCAD 2026, 2026-09-23: [200,200,0, 4200,200,0, ...]) which the backend reads two at a time, so three closed ms.AddPolyline outlines on WALLS reported points like [[0,0],[0,8600],[0,0],[8600,5400],...] and rooms_detect answered five faces of 2.6-6.9 m2 for two 20 m2 rooms. Headless, the same POLYLINE arrives with no points and was dropped without a word; so were walls inside a block reference. The reader now reads LINE and the lightweight polyline only (LWPOLYLINE, and the live AcDbPolyline reported POLYLINE), and lists every other entity that could bound a room in `skipped` with its handle, type and reason: arcs and bulged edges as before, old-style polylines (2DPOLYLINE/3DPOLYLINE live, a vertex-less POLYLINE headless), block references (INSERT / BLOCKREFERENCE) and anything else not a line. Text, dimensions, leaders, points and hatches bound nothing and are passed over. Re-run live on AutoCAD 2026 through the real com_backend._entity_info rows: count 0 with the three polylines in skipped (was 5 wrong rooms); with a lightweight pair and a block added, count 1 at 20 000 000 mm2 plus the block reference in skipped. The backend's stride is left alone (shared file, other readers); this reader no longer trusts it. label_room refused a reused id but not a reused face: a second label in a labelled room (a retry after a transport timeout) wrote a second record with the same area, which a schedule total counts twice. It is now refused before anything is drawn, naming the record and label already there. rooms_detect reports `labels` (every record in the face; `label` stays the first) and `label_conflicts` for a face holding more than one. No new ActiveX member is called. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/arch/rooms.py | 164 ++++++++++++++++++++++++------- server.py | 25 +++-- tests/test_arch_rooms.py | 197 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 340 insertions(+), 46 deletions(-) diff --git a/engineering/arch/rooms.py b/engineering/arch/rooms.py index 7064881..ab403f4 100644 --- a/engineering/arch/rooms.py +++ b/engineering/arch/rooms.py @@ -14,10 +14,14 @@ memory only, never drawn - which is the convention a net floor area is measured to (the wall face, not the reveal). A plan without our records has no such lines, and `rooms_detect` says so rather than guessing where a door was. -`rooms_detect` runs the same finder on any lines and polylines - a foreign -plan - and reports each face with a confidence, the way `pid_graph` does: -1.0 when every edge of the face lies on our wall layer, 0.6 when any edge is a -plain line from somewhere else. It never modifies the drawing. +`rooms_detect` runs the same finder on any lines and lightweight polylines - +a foreign plan - and reports each face with a confidence, the way `pid_graph` +does: 1.0 when every edge of the face lies on our wall layer, 0.6 when any +edge is a plain line from somewhere else. It never modifies the drawing. What +it cannot read - an arc, an old-style POLYLINE, walls inside a block +reference - is listed in ``skipped`` with its handle, never dropped and never +approximated, so a room it could not close is a room the answer says it +missed, not a wrong area. DECLARED, not standard: @@ -89,6 +93,40 @@ WALL_LAYER_KEYWORDS = ("WALL", "DUVAR") #: Entity types that bound a region with a curve; the finder reads straight #: segments only, so these are reported as skipped, never approximated. CURVE_TYPES = frozenset({"ARC", "CIRCLE", "ELLIPSE", "SPLINE"}) +#: Polylines whose ``points`` are the vertices on both engines: the headless +#: LWPOLYLINE, and the live AcDbPolyline / AcDbLWPolyline (reported POLYLINE / +#: LWPOLYLINE). A headless POLYLINE is the old-style one - it arrives with no +#: ``points`` at all, and is skipped by that test. +LIGHT_POLYLINE_TYPES = frozenset({"LWPOLYLINE", "POLYLINE"}) +#: Old-style polylines whose ``points`` are NOT to be trusted: the live +#: AcDb2dPolyline hands its Coordinates over as x, y, z triples, which the +#: backend reads two at a time (measured on AutoCAD 2026: a 4000 x 5000 outline +#: came back as [[200, 200], [0, 4200], [200, 0], ...]), so a room read off them +#: is a wrong number, not a missing one. Skipped, by name. +HEAVY_POLYLINE_TYPES = frozenset({"2DPOLYLINE", "3DPOLYLINE", "POLYFACEMESH", "POLYGONMESH"}) +#: Block references on both engines (headless INSERT, live AcDbBlockReference). +BLOCK_REFERENCE_TYPES = frozenset({"INSERT", "BLOCKREFERENCE", "MINSERTBLOCK"}) +#: Annotation and fills: they sit on wall layers (a poche hatch, a wall tag) +#: without bounding anything, so they are passed over without a skipped entry. +#: Every DIMENSION kind (the live engine names them ROTATEDDIMENSION, ...) is +#: matched by name below. +NON_BOUNDARY_TYPES = frozenset( + { + "TEXT", + "MTEXT", + "ATTDEF", + "ATTRIBUTEDEFINITION", + "ATTRIB", + "POINT", + "HATCH", + "LEADER", + "MLEADER", + "MULTILEADER", + "TOLERANCE", + "FCF", + } +) +_ARC_REASON = "the face finder reads straight segments only and does not approximate an arc" def _check_lang(lang: str) -> str: @@ -283,23 +321,39 @@ async def read_records(backend: AutoCADBackend) -> dict: async def read_segments(backend: AutoCADBackend, layers) -> tuple[list, list[dict]]: """Straight segments on ``layers`` as ``(p1, p2, layer)``, and what was skipped. - LINE and (LW)POLYLINE are read by their properties, which is the same on - both engines (the live one names a polyline POLYLINE, the headless one - LWPOLYLINE). A bulged polyline edge or an ARC/CIRCLE/ELLIPSE/SPLINE is an - arc, and the finder reads straight segments only: it is listed in the - second return value with its handle, never flattened into chords. + Read: LINE, and the lightweight polyline by its ``points`` (the live engine + names it POLYLINE, the headless one LWPOLYLINE). Everything else that can + bound a room is listed in the second return value with its handle, type and + reason - never dropped, never guessed: + + * an ARC/CIRCLE/ELLIPSE/SPLINE or a bulged polyline edge is an arc, and the + finder reads straight segments only - it is never flattened into chords; + * an old-style POLYLINE (``HEAVY_POLYLINE_TYPES`` live; a headless POLYLINE + reports no vertices) is not read, because its vertices do not reach this + reader correctly on either engine; + * a block reference is not exploded: walls inside a block are not read; + * any other type is listed as not a line. + + Text, dimensions, leaders, points and hatches (``NON_BOUNDARY_TYPES``) + bound nothing and are passed over without an entry. """ from engineering.sheet.bom import all_entities # lazy: sheet imports stay out of arch segments: list = [] skipped: list[dict] = [] + + def skip(entity, kind: str, reason: str) -> None: + skipped.append({"handle": entity.handle, "type": kind, "reason": reason}) + for layer in layers: for entity in await all_entities(backend, layer=layer): props = entity.properties or {} kind = str(entity.type).upper() - if "start" in props and "end" in props and "center" not in props: + if kind in NON_BOUNDARY_TYPES or "DIMENSION" in kind: + continue + if kind == "LINE" and "start" in props and "end" in props: segments.append((tuple(props["start"][:2]), tuple(props["end"][:2]), entity.layer)) - elif "points" in props: + elif kind in LIGHT_POLYLINE_TYPES and props.get("points"): points = [tuple(p[:2]) for p in props["points"]] bulges = list(props.get("bulges") or []) count = len(points) @@ -310,22 +364,29 @@ async def read_segments(backend: AutoCADBackend, layers) -> tuple[list, list[dic continue segments.append((points[i], points[(i + 1) % count], entity.layer)) if arcs: - skipped.append( - { - "handle": entity.handle, - "type": kind, - "reason": f"{arcs} arc segment(s): the face finder reads straight " - "segments only and does not approximate an arc", - } - ) + skip(entity, kind, f"{arcs} arc segment(s): {_ARC_REASON}") elif kind in CURVE_TYPES: - skipped.append( - { - "handle": entity.handle, - "type": kind, - "reason": "a curve: the face finder reads straight segments only " - "and does not approximate an arc", - } + skip(entity, kind, f"a curve: {_ARC_REASON}") + elif kind in HEAVY_POLYLINE_TYPES or kind == "POLYLINE": + skip( + entity, + kind, + "an old-style polyline: its vertices are not read reliably by this " + "reader on either engine, so its edges are not counted; convert it to a " + "lightweight polyline (CONVERTPOLY) or explode it to lines", + ) + elif kind in BLOCK_REFERENCE_TYPES: + skip( + entity, + kind, + "a block reference: walls inside a block are not read; explode it " + "(block_explode) to count them", + ) + else: + skip( + entity, + kind, + "not a LINE or a lightweight polyline: the face finder does not read it", ) return segments, skipped @@ -342,8 +403,10 @@ async def label_room( Refused before anything is drawn: an ``area`` in the request (areas are measured, never typed), an unknown ``lang``, a non-positive ``scale``, an ``id`` already labelled on the drawing, a point that lies in no closed face - of the wall layer, and a point inside a wall body. ``id`` defaults to the - first free ``R``. + of the wall layer, a point inside a wall body, and a point in a face that + already holds a room record (one face, one room: a second record would be + counted twice by every schedule total). ``id`` defaults to the first free + ``R``. """ if not isinstance(room_dict, dict): raise ValueError( @@ -391,6 +454,18 @@ async def label_room( f"room.at: ({x:g}, {y:g}) lies inside a wall body (a face {width:.0f} mm wide on " f"average, under the {MIN_ROOM_WIDTH:.0f} mm a room is taken to need), not in a room" ) + # One face is one room: a second record on it would be a second area in + # every schedule total (a retried call after a timeout is the usual way in). + already = [r for r in records["rooms"] if face_containing(faces, r.at) is face] + if already: + held = ", ".join( + f"{r.id} {r.name!r} (label {records['room_handles'][r.id]})" for r in already + ) + raise ValueError( + f"room.at: ({x:g}, {y:g}) lies in a room that is already labelled - {held}; " + "one face carries one room record, so its area is counted once. Delete that " + "label to relabel the room, or place this one in another room" + ) measured = replace(room, area=face.area) from engineering.mech.draw import draw_prims # lazy: mech.draw imports the arch layer set @@ -437,6 +512,12 @@ async def rooms_detect( the ones it does. Our own opening records, when there are any, close door and window gaps exactly as `label_room` does; a foreign plan's gaps stay open, and two rooms joined by an open doorway read as one face. + + Each room reports ``labels`` - every room record whose point lies in its + face - and ``label``, the first of them or None. A face carrying more than + one record is listed in ``label_conflicts``, because a schedule reading the + records would count its area once per record. What `read_segments` could + not read is in ``skipped``, with its handle and why. """ min_area = _positive(min_area, "min_area") tol = _positive(tol, "tol") @@ -469,18 +550,25 @@ async def rooms_detect( rooms = detect_rooms([*segments, *extra], min_area=min_area, tol=tol) out: list[dict] = [] + conflicts: list[dict] = [] for room in rooms: face = Face( loop=room["loop"], area=room["area"], centroid=room["centroid"], holes=room["holes"] ) - label = next( - ( - {"id": r.id, "name": r.name, "number": r.number, "area": r.area} - for r in records["rooms"] - if face_containing((face,), r.at) is not None - ), - None, - ) + labels = [ + {"id": r.id, "name": r.name, "number": r.number, "area": r.area} + for r in records["rooms"] + if face_containing((face,), r.at) is not None + ] + if len(labels) > 1: + conflicts.append( + { + "labels": [label["id"] for label in labels], + "centroid": [room["centroid"][0], room["centroid"][1]], + "reason": f"one face carries {len(labels)} room records; a schedule " + f"would count its {room['area'] / 1.0e6:.2f} m2 {len(labels)} times", + } + ) out.append( { "loop": _wcs_loop(room["loop"]), @@ -489,7 +577,8 @@ async def rooms_detect( "area_m2": room["area"] / 1.0e6, "centroid": [room["centroid"][0], room["centroid"][1]], "confidence": room["confidence"], - "label": label, + "label": labels[0] if labels else None, + "labels": labels, } ) return { @@ -501,6 +590,7 @@ async def rooms_detect( "count": len(out), "rooms": out, "confidence_min": min((room["confidence"] for room in out), default=None), + "label_conflicts": conflicts, "skipped": skipped, "omitted": list(omitted), "min_area": min_area, diff --git a/server.py b/server.py index 8513062..6c6524d 100644 --- a/server.py +++ b/server.py @@ -9818,7 +9818,8 @@ async def arch_room( """Label the room around (x, y) with its name, number and MEASURED area. The area is never typed: the tool reads the wall faces on the `wall` layer - of the arch set (LINE and polyline, both engines), finds the closed face + of the arch set (LINE and lightweight polyline, both engines; anything else + there is listed in `skipped`), finds the closed face the point lies in and writes that face's area - net of the wall thickness and of any column inside it - into the label and into an `ACADMCP_ARCH` room record on the name text, which `arch_schedule` reads. A door or window @@ -9828,8 +9829,10 @@ async def arch_room( Refused before anything is drawn: an unknown `lang` (named with en, tr), a `number`/`name` the room model refuses, a `room_id` already labelled on the drawing, a point in no closed face of the walls (draw or close them first, - or read a plan drawn by other means with `arch_rooms_detect`), and a point - inside a wall body. + or read a plan drawn by other means with `arch_rooms_detect`), a point + inside a wall body, and a point in a room that already carries a label (one + face, one room record - a retried call does not add a second area to the + schedule). """ from engineering.arch.rooms import label_room @@ -9869,17 +9872,21 @@ async def arch_rooms_detect( ) -> dict: """Read the rooms of a plan - ours or a foreign one made of plain lines. - Every LINE and polyline on the layers is split where lines cross or touch, - and the closed faces they bound are measured (mm², net of nested pieces). + Every LINE and lightweight polyline on the layers is split where lines + cross or touch, and the closed faces they bound are measured (mm², net of + nested pieces). Faces under `min_area`, and thin faces (mean width under 600 mm - wall bodies and reveals, this reader's heuristic) are left out. Each room reports `confidence`: 1.0 when every edge lies on the arch wall layer, 0.6 when any edge is a plain line from elsewhere - read `confidence_min` before - trusting a foreign plan. A room label already on the drawing is reported - with the face it sits in. + trusting a foreign plan. Room labels already on the drawing are reported + in `labels` with the face they sit in; a face with more than one is listed + in `label_conflicts`. - Never modifies the drawing. Arcs and bulged polyline edges are listed in - `skipped`, never flattened into chords. A foreign plan's open doorways are + Never modifies the drawing. Arcs and bulged polyline edges, old-style + POLYLINEs and block references are listed in `skipped` with their handle + and reason - never flattened into chords, never dropped silently. Read + `skipped` before trusting `count`. A foreign plan's open doorways are not closed - two rooms joined by one read as one face. Refused: a named layer the drawing does not have (with the list of the ones it has), and no layer matching WALL or DUVAR when `layers` is omitted. diff --git a/tests/test_arch_rooms.py b/tests/test_arch_rooms.py index e795c1c..e5c22c0 100644 --- a/tests/test_arch_rooms.py +++ b/tests/test_arch_rooms.py @@ -347,3 +347,200 @@ async def test_an_arc_on_a_wall_layer_is_reported_skipped_never_approximated(bac result = await rooms_detect(backend) assert [r["area"] for r in result["rooms"]] == [20_000_000.0] assert [s["handle"] for s in result["skipped"]] == [arc.handle] + + +# ── what the reader cannot read is skipped, by name ───────────────────────── + + +def outline(x0, y0, x1, y1): + return [(x0, y0), (x1, y0), (x1, y1), (x0, y1)] + + +@pytest.mark.asyncio +async def test_an_old_style_polyline_is_skipped_by_name_not_dropped(backend): + # one room of old-style 2D POLYLINEs next to one of lightweight polylines; + # the headless engine reports the old-style kind with no vertices at all + msp = backend._doc.modelspace() + heavy = [ + msp.add_polyline2d(outline(*box), close=True, dxfattribs={"layer": "WALLS"}).dxf.handle + for box in ((0.0, 0.0, 4400.0, 5400.0), (200.0, 200.0, 4200.0, 5200.0)) + ] + for box in ((10000.0, 0.0, 14400.0, 5400.0), (10200.0, 200.0, 14200.0, 5200.0)): + await backend.entity_create_polyline([list(p) for p in outline(*box)], True, layer="WALLS") + result = await rooms_detect(backend) + assert [r["area"] for r in result["rooms"]] == [20_000_000.0] + assert [(s["handle"], s["type"]) for s in result["skipped"]] == [ + (heavy[0], "POLYLINE"), + (heavy[1], "POLYLINE"), + ] + assert "old-style polyline" in result["skipped"][0]["reason"] + + +@pytest.mark.asyncio +async def test_walls_inside_a_block_reference_are_skipped_and_annotation_is_passed_over( + backend, +): + await draw_lines(backend, rect(0.0, 0.0, 4000.0, 5000.0), "WALLS") + block = backend._doc.blocks.new("ROOMBLK") + for (x1, y1), (x2, y2) in [ + *rect(0.0, 0.0, 4400.0, 5400.0), + *rect(200.0, 200.0, 4200.0, 5200.0), + ]: + block.add_line((x1, y1), (x2, y2)) + insert = backend._doc.modelspace().add_blockref( + "ROOMBLK", (20000.0, 0.0), dxfattribs={"layer": "WALLS"} + ) + await backend.entity_create_text("W1", 100.0, 100.0, 100.0, layer="WALLS") + result = await rooms_detect(backend) + assert [r["area"] for r in result["rooms"]] == [20_000_000.0] + assert [(s["handle"], s["type"]) for s in result["skipped"]] == [(insert.dxf.handle, "INSERT")] + assert "walls inside a block are not read" in result["skipped"][0]["reason"] + + +class LiveRows: + """A backend that answers with the EntityInfo rows a live seat produced. + + The rows are the ones `backends.com_backend._entity_info` returned on + AutoCAD 2026 (2026-09-23) for three closed ``ModelSpace.AddPolyline`` + outlines on WALLS - outer 0..8600 x 0..5400, rooms 200..4200 and + 4400..8400 x 200..5200 - whose ``Coordinates`` came back as x, y, z + triples and were read two at a time; plus an ``AddLightWeightPolyline`` + pair (AcDbPolyline, reported POLYLINE) and a block reference. Read + through, the old-style rows gave five faces of 2.6 to 6.9 m². + """ + + def __init__(self): + from backends.base import EntityInfo, LayerInfo + + def row(handle, kind, points=None): + props = {} if points is None else {"points": points, "closed": True} + if points is not None: + props["bulges"] = [0.0] * len(points) + return EntityInfo(handle, kind, "WALLS", 256, "ByLayer", True, props) + + self.rows = [ + row( + "2A1", + "2DPOLYLINE", + [ + [0.0, 0.0], + [0.0, 8600.0], + [0.0, 0.0], + [8600.0, 5400.0], + [0.0, 0.0], + [5400.0, 0.0], + ], + ), + row( + "2A2", + "2DPOLYLINE", + [ + [200.0, 200.0], + [0.0, 4200.0], + [200.0, 0.0], + [4200.0, 5200.0], + [0.0, 200.0], + [5200.0, 0.0], + ], + ), + row( + "2A3", + "2DPOLYLINE", + [ + [4400.0, 200.0], + [0.0, 8400.0], + [200.0, 0.0], + [8400.0, 5200.0], + [0.0, 4400.0], + [5200.0, 0.0], + ], + ), + row("2A4", "POLYLINE", [list(p) for p in outline(10000.0, 0.0, 14400.0, 5400.0)]), + row("2A5", "POLYLINE", [list(p) for p in outline(10200.0, 200.0, 14200.0, 5200.0)]), + row("2A6", "BLOCKREFERENCE"), + ] + self.layers = [ + LayerInfo(name, 7, "Continuous", 0.25, True, False, False, name == "0") + for name in ("0", "WALLS") + ] + + async def layer_list(self): + return self.layers + + async def entity_list(self, type_filter=None, layer_filter=None, limit=200, offset=0): + rows = [r for r in self.rows if layer_filter is None or r.layer == layer_filter] + return rows[offset : offset + limit] + + async def entity_get_xdata(self, handle, app_id): + return {"xdata": {}} + + +@pytest.mark.asyncio +async def test_live_old_style_polylines_are_skipped_not_read_as_their_garbled_points(): + result = await rooms_detect(LiveRows()) + # only the lightweight pair is a room; the garbled rows make no faces at all + assert [r["area"] for r in result["rooms"]] == [20_000_000.0] + assert result["confidence_min"] == CONFIDENCE_FOREIGN + assert [(s["handle"], s["type"]) for s in result["skipped"]] == [ + ("2A1", "2DPOLYLINE"), + ("2A2", "2DPOLYLINE"), + ("2A3", "2DPOLYLINE"), + ("2A6", "BLOCKREFERENCE"), + ] + + +# ── one face, one room record ─────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_a_second_label_in_a_labelled_room_is_refused_and_draws_nothing(backend): + await draw_lines( + backend, [*rect(0.0, 0.0, 4400.0, 5400.0), *rect(200.0, 200.0, 4200.0, 5200.0)], WALL + ) + first = await label_room(backend, {"name": "LIVING", "at": [1000.0, 3000.0]}) + before = await backend.entity_count() + with pytest.raises( + ValueError, + match=re.escape( + f"room.at: (2000, 3000) lies in a room that is already labelled - " + f"R1 'LIVING' (label {first['handle']})" + ), + ): + await label_room(backend, {"name": "LIVING2", "at": [2000.0, 3000.0]}) + assert await backend.entity_count() == before + result = await rooms_detect(backend) + assert [r["labels"] for r in result["rooms"]] == [ + [{"id": "R1", "name": "LIVING", "number": None, "area": 20_000_000.0}] + ] + assert result["label_conflicts"] == [] + + +@pytest.mark.asyncio +async def test_the_other_room_can_still_be_labelled(backend): + await draw_lines(backend, two_rooms(), WALL) + await label_room(backend, {"name": "A", "at": [2200.0, 2700.0]}) + second = await label_room(backend, {"name": "B", "at": [6400.0, 2700.0]}) + assert second["room"]["id"] == "R2" + assert second["room"]["area"] == pytest.approx(20_000_000.0, abs=EPS) + + +@pytest.mark.asyncio +async def test_rooms_detect_reports_every_record_in_a_face_and_the_conflict(backend): + # two records already on one face (written by other means than arch_room) + await draw_lines(backend, two_rooms(), WALL) + for rid in ("R1", "R2"): + room = Room(id=rid, name=rid, number=None, at=(2200.0, 2700.0), area=20_000_000.0) + await write_record(backend, room, ROOM) + result = await rooms_detect(backend) + first, second = result["rooms"] + assert [label["id"] for label in first["labels"]] == ["R1", "R2"] + assert first["label"]["id"] == "R1" + assert second["labels"] == [] and second["label"] is None + assert result["label_conflicts"] == [ + { + "labels": ["R1", "R2"], + "centroid": [2200.0, 2700.0], + "reason": "one face carries 2 room records; a schedule would count its " + "20.00 m2 2 times", + } + ]