From bfc950a94833da53ac86f26e75cd7c76579e385d Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Sat, 11 Jul 2026 15:27:43 +0300 Subject: [PATCH] Refuse cyclic aggregation in aggregate.assign_object (#7154) Aggregating a product under relating_object makes the product an ancestor of relating_object. If a product is relating_object itself, or is already an ancestor of it, the new IfcRelAggregates closes a loop in the decomposition tree. Traversing that loop later (for example resolving a relative placement via util.placement.get_local_placement) recurses without end and raises RecursionError; without placements it silently writes an invalid cyclic model. Before writing anything, walk the aggregate ancestor chain of relating_object (including itself, with a visited set so a pre-existing bad cycle is still safe) and raise a clear ValueError if any product is in it, rather than proceeding. Fixing this in the api protects every caller, including the Bonsai operator. Verified: self-aggregation, a 2-node cycle, and a deeper cycle now raise a clean ValueError instead of RecursionError; a normal aggregate of a distinct element under a distinct aggregate still works, and the existing test/api/aggregate suite (20 tests) passes. Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Opus 4.8 --- .../api/aggregate/assign_object.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/ifcopenshell-python/ifcopenshell/api/aggregate/assign_object.py b/src/ifcopenshell-python/ifcopenshell/api/aggregate/assign_object.py index a7e3e403d8..40d0660ee1 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/aggregate/assign_object.py +++ b/src/ifcopenshell-python/ifcopenshell/api/aggregate/assign_object.py @@ -88,6 +88,24 @@ def assign_object( return products_set = set(products) + + # Guard against creating a cyclic aggregation. Aggregating a product under + # relating_object makes the product an ancestor of relating_object, so if a + # product is relating_object itself or already an ancestor of it, the + # relationship would form a loop. Such a cycle later causes infinite + # recursion (e.g. RecursionError) when placements or the decomposition tree + # are traversed, so refuse it up front. See issue #7154. + ancestor = relating_object + ancestors: set[ifcopenshell.entity_instance] = set() + while ancestor is not None and ancestor not in ancestors: + ancestors.add(ancestor) + ancestor = ifcopenshell.util.element.get_aggregate(ancestor) + cycling = products_set & ancestors + if cycling: + raise ValueError( + "Cannot aggregate an object under itself or one of its descendants " + f"(would create a cyclic aggregation): {cycling}" + ) is_decomposed_by = next((i for i in relating_object.IsDecomposedBy if i.is_a("IfcRelAggregates")), None) previous_aggregates_rels: set[ifcopenshell.entity_instance] = set()