From d1f9e5243efb09cf84c21b567b8ebee8a23ec5f0 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 24 Jul 2026 10:31:44 +0300 Subject: [PATCH] Fix ci-bonsai-daily: ProjectLibraryData duplicate parent-library enum entry (#8573) * Fix ci-bonsai-daily: ProjectLibraryData duplicate parent-library enum parent_libraries_enum() adds an explicit entry for get_root_context(), then loops over cls.data["project_libraries"] (all IfcProjectLibrary entities) and appends each. For a library-only file (no IfcProject), get_root_context falls back to the top-level IfcProjectLibrary itself, so the root is appended twice with the same enum key (its STEP id), which Blender EnumProperty requires to be unique -> the data load asserts. Normal project files are unaffected (root is an IfcProject whose id never collides with a library id). Skip library_id == root.id() in the loop (dedup by id, the colliding key). Verified in headless Blender: test_project_library_data.py::TestLibraryOnlyFile goes from 1 failed / 5 passed to 6 passed. This change was made with the assistance of an AI tool. Co-Authored-By: Claude Fable 5 * Bonsai: repair library files missing the required IfcProject, not just the symptom Per the IFC Project Context concept template, every project data set (library files included) shall contain exactly one IfcProject, and IfcProjectLibrary instances are assigned to it via IfcRelDeclares. There is no such thing as a spec-valid file rooted on IfcProjectLibrary alone. get_root_context() (added in 260a387069, #8184) treated a missing IfcProject as license to use the top-level IfcProjectLibrary as the file's root context instead. That invalid premise is why project_libraries() (which walks every IfcProjectLibrary, root included) then re-added that same entity, producing the duplicate, colliding enum key this PR originally papered over with a dedup guard. Add tool.Project.ensure_project_context(), which repairs a file missing IfcProject by creating one and declaring the file's root-level IfcProjectLibrary instances to it, and tool.Project.open_library_file(), which opens a library file through that repair. Route all three IfcStore.library_file load sites in SelectLibraryFile through it. Downstream code (get_root_context, ProjectLibraryData, RefreshLibrary, AddProjectLibrary) now always operates on a spec-valid model, so the duplicate enum entry cannot occur; the previous one-line dedup guard in parent_libraries_enum() is kept only as cheap defense in depth for callers that bypass the load-time repair, not as the fix. Rework test_project_library_data.py: the previous _make_library_only_file() fixture built an invalid library-only model and asserted that as correct behaviour. Replace it with a spec-valid fixture (IfcProject + IfcProjectLibrary declared to it) for the downstream tests, and a malformed fixture used only to exercise the new repair path. Verified live in headless Blender (isolated profile): reproduced the original duplicate-enum-key failure mode, then confirmed ensure_project_context/ open_library_file repair a malformed file and ProjectLibraryData, refresh_library and add_project_library all operate correctly on the result, with no duplicate keys and no regression on already-valid files or IFC2X3. This change was made with the assistance of an AI tool. * Bonsai: stop supporting library-only files, do not repair them Per Moult's feedback: if the IFC is invalid, our default position is to not support it, not to patch around it. A library file with no IfcProject is invalid IFC (Project Context concept template requires exactly one IfcProject), and it is not ubiquitous: every library file bonsai ships under bim/data/libraries has an IfcProject with the IfcProjectLibrary declared to it via IfcRelDeclares. The single #8183 report is an outlier, not a common authoring pattern worth accommodating. Remove tool.Project.ensure_project_context() and open_library_file() (the load-time repair added in the previous commit here) and revert SelectLibraryFile's three load sites to plain ifcopenshell.open. Simplify get_root_context() back to returning ifc_file.by_type("IfcProject")[0] directly, no IfcProjectLibrary fallback: a file without IfcProject now raises IndexError instead of being silently treated as valid. AddProjectLibrary's nest-under-library branch is now dead code (root_context is always an IfcProject) and is removed. The one-line enum dedup guard from the original commit here is also removed: since get_root_context can only return an IfcProject or raise, an IfcProject id can never collide with a library id, so the guard has nothing left to guard against. Rework test_project_library_data.py: drop the invalid _make_library_only_file fixture and its tests, which asserted an unsupported model as correct behaviour. Replace with a single spec-valid fixture matching bonsai's own shipped library files (IfcProject + IfcProjectLibrary declared to it), used for the ci-bonsai-daily regression test and the refresh/add-library operators, plus one explicit test that get_root_context raises for a file without IfcProject, documenting that this input is intentionally unsupported rather than silently tolerated. Verified live in headless Blender (isolated profile, source-loaded, never the real profile): confirmed the removed methods are gone, that a library-only file now raises instead of being handled, that ProjectLibraryData/refresh_library/add_project_library all work correctly on a spec-valid model with unique enum keys, and spot-checked that every library file under bim/data/libraries already has an IfcProject. This change was made with the assistance of an AI tool. * Bonsai: inline get_root_context, trim docstrings, confirm get_parent_library unchanged Per Moult's round 3 review. get_root_context added nothing over ifc_file.by_type("IfcProject")[0], which is guaranteed by the IFC Project Context concept template; remove it and inline the call at its three sites (operator.py's RefreshLibrary and AddProjectLibrary, data.py's parent_libraries_enum). Trim the get_parent_library docstring to one line; its logic is untouched by this PR, byte for byte identical to origin/v0.8.0, and still returns None only when project_library has neither Nests nor HasContext, never for a library declared directly to IfcProject. Rework test_project_library_data.py to match: replace the two get_root_context-specific tests with one that exercises the real call site (ProjectLibraryData.parent_libraries_enum raising IndexError for a file without IfcProject), and add an explicit test that get_parent_library returns None for a genuinely orphaned library. Also drop a long inline comment that restated what the test body already shows. Verified live in headless Blender (isolated profile, source-loaded, never the real profile): all 17 test/bim/module/project tests pass, including the new get_parent_library None-for-orphan case. Ran the full test/bim suite before and after on the identical harness: 82 failed/1335 passed both times, same failing tests (all pre-existing, unrelated to this module). This change was made with the assistance of an AI tool. * Bonsai: fix EditProjectLibrary leaving stale declarations after reparenting Per Moult's round 4 review. The assertion change (get_parent_library(root) now returns the IfcProject instead of None) is correct: in the old library-only test model a top-level library had neither IfcRelNests nor IfcRelDeclares, so None meant "top level". In the new spec-valid model a top-level library is always declared to the guaranteed IfcProject via IfcRelDeclares, so get_parent_library correctly resolves it through the HasContext branch instead of falling through to None. get_project_hierarchy already keys top-level libraries under the project for exactly this reason, so the library tree still renders correctly. Auditing every caller found one real bug in EditProjectLibrary, which Gorgious56 originally wrote for the library-only model. Its move-library logic assumed a top-level library (previous_parent_library is None) needed no cleanup before nesting it under a new parent, and that unnesting a library back to the project needed no new relationship because it was "already assigned by default". Both assumptions relied on a top-level library never actually holding a IfcRelDeclares, which is no longer true. Reproduced live: moving a project-declared library under another library left its old IfcRelDeclares dangling alongside the new IfcRelNests (an invalid double parentage), and moving a nested library back to the project left it with neither relationship, orphaning it out of the tree entirely. Fixed by tearing down whichever of IfcRelDeclares/IfcRelNests the library previously had before establishing whichever one the new parent requires, instead of assuming which prior state applies. Added tests: get_parent_library resolving a nested sub-library to its library parent (the third contract case alongside project-declared and orphaned), and both EditProjectLibrary reparenting directions, which fail without the operator.py fix and pass with it. Verified live in headless Blender (isolated profile, source-loaded, never the real profile): all 20 test/bim/module/project tests pass. Ran the full test/bim suite before and after on the identical harness: 123 failed/1294 passed before, 123 failed/1297 passed after, identical failing test names in both runs (diffed), the extra 3 passes are the new tests above. This change was made with the assistance of an AI tool. --------- Co-authored-by: Claude Fable 5 --- src/bonsai/bonsai/bim/module/project/data.py | 2 +- .../bonsai/bim/module/project/operator.py | 32 ++-- src/bonsai/bonsai/tool/project.py | 19 +-- .../project/test_project_library_data.py | 139 ++++++++++++++---- 4 files changed, 127 insertions(+), 65 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/project/data.py b/src/bonsai/bonsai/bim/module/project/data.py index db64041a89..8ad010bc25 100644 --- a/src/bonsai/bonsai/bim/module/project/data.py +++ b/src/bonsai/bonsai/bim/module/project/data.py @@ -162,7 +162,7 @@ class ProjectLibraryData: library_file = IfcStore.library_file if library_file is None or library_file.schema == "IFC2X3": return results - root = tool.Project.get_root_context(library_file) + root = library_file.by_type("IfcProject")[0] results.append((str(root.id()), f"{root.is_a()} {root.Name or 'Unnamed'}", root.Description or "")) for library_id, data in cls.data["project_libraries"].items(): results.append((str(library_id), data["Name"] or "Unnamed", data["Description"] or "")) diff --git a/src/bonsai/bonsai/bim/module/project/operator.py b/src/bonsai/bonsai/bim/module/project/operator.py index cb6a27c52b..efe36b0c78 100644 --- a/src/bonsai/bonsai/bim/module/project/operator.py +++ b/src/bonsai/bonsai/bim/module/project/operator.py @@ -281,7 +281,7 @@ class RefreshLibrary(bpy.types.Operator): elements = {e for e in elements if not tool.Project.is_element_assigned_to_project_library(e, rels)} self.props.add_library_project_library("Unassigned", len(elements), 0, False) - root_context = tool.Project.get_root_context(library_file) + root_context = library_file.by_type("IfcProject")[0] hierarchy = tool.Project.get_project_hierarchy(library_file) tool.Project.load_project_libraries_to_ui(root_context, hierarchy) return {"FINISHED"} @@ -761,21 +761,22 @@ class EditProjectLibrary(bpy.types.Operator): attributes = bonsai.bim.helper.export_attributes(props.project_library_attributes) ifcopenshell.api.attribute.edit_attributes(library_file, project_library, attributes) - # Update parent library. + # Update parent library. Tear down the old IfcRelDeclares/IfcRelNests before + # creating the new one; a library must have exactly one of the two, never both. previous_parent_library = tool.Project.get_parent_library(project_library) new_parent_library = library_file.by_id(int(props.parent_library)) if previous_parent_library != new_parent_library: - if previous_parent_library is None: - # Edited library was a root in a library-only file; nest it under the new parent. + if previous_parent_library is not None: + if previous_parent_library.is_a("IfcProject"): + ifcopenshell.api.project.unassign_declaration( + library_file, [project_library], previous_parent_library + ) + else: + ifcopenshell.api.nest.unassign_object(library_file, [project_library]) + if new_parent_library.is_a("IfcProject"): + ifcopenshell.api.project.assign_declaration(library_file, [project_library], new_parent_library) + else: ifcopenshell.api.nest.assign_object(library_file, [project_library], new_parent_library) - elif previous_parent_library.is_a("IfcProject"): - # Then new one is IfcProjectLibrary. - ifcopenshell.api.nest.assign_object(library_file, [project_library], new_parent_library) - else: # Previous is IfcProjectLibrary. - ifcopenshell.api.nest.unassign_object(library_file, [project_library]) - # If new one is IfcProject, then it's already assigned by default. - if new_parent_library.is_a("IfcProjectLibrary"): - ifcopenshell.api.nest.assign_object(library_file, [project_library], new_parent_library) props.is_editing_project_library = False bpy.ops.bim.refresh_library() @@ -809,12 +810,9 @@ class AddProjectLibrary(bpy.types.Operator): props = tool.Project.get_project_props() library_file = IfcStore.library_file assert library_file - root_context = tool.Project.get_root_context(library_file) + root_context = library_file.by_type("IfcProject")[0] project_library = ifcopenshell.api.root.create_entity(library_file, "IfcProjectLibrary") - if root_context.is_a("IfcProject"): - ifcopenshell.api.project.assign_declaration(library_file, [project_library], root_context) - else: - ifcopenshell.api.nest.assign_object(library_file, [project_library], root_context) + ifcopenshell.api.project.assign_declaration(library_file, [project_library], root_context) ProjectLibraryData.load() # Update enum. props.selected_project_library = str(project_library.id()) props.is_editing_project_library = True diff --git a/src/bonsai/bonsai/tool/project.py b/src/bonsai/bonsai/tool/project.py index 1d7a2dc2e0..f65ddd1519 100644 --- a/src/bonsai/bonsai/tool/project.py +++ b/src/bonsai/bonsai/tool/project.py @@ -391,29 +391,14 @@ class Project(bonsai.core.tool.Project): def get_parent_library( cls, project_library: ifcopenshell.entity_instance ) -> Union[ifcopenshell.entity_instance, None]: - """Return the IfcContext that declares or nests ``project_library``. - - Returns ``None`` when ``project_library`` is itself the root of a - library-only file (no IfcRelNests, no IfcRelDeclares). - """ + """Return the IfcContext that declares or nests ``project_library``, or ``None`` + if neither relationship is present.""" if nests := project_library.Nests: return nests[0].RelatingObject if has_context := project_library.HasContext: return has_context[0].RelatingContext return None - @classmethod - def get_root_context(cls, ifc_file: ifcopenshell.file) -> ifcopenshell.entity_instance: - """Return the file's root IfcContext. - - Prefers IfcProject if present, otherwise falls back to IfcProjectLibrary — - library-only files are valid per IFC4+ and contain no IfcProject. Caller is - responsible for the IFC2X3 guard; IfcContext does not exist in that schema. - """ - if projects := ifc_file.by_type("IfcProject"): - return projects[0] - return ifc_file.by_type("IfcProjectLibrary")[0] - @classmethod def get_project_hierarchy(cls, ifc_file: ifcopenshell.file) -> HiearchyDict: """Get project hierarchy in the following form: diff --git a/src/bonsai/test/bim/module/project/test_project_library_data.py b/src/bonsai/test/bim/module/project/test_project_library_data.py index 08f5573b00..df9192aa6d 100644 --- a/src/bonsai/test/bim/module/project/test_project_library_data.py +++ b/src/bonsai/test/bim/module/project/test_project_library_data.py @@ -32,64 +32,91 @@ from test.bim.bootstrap import NewIfc pytestmark = pytest.mark.project -def _make_library_only_file(*, with_child: bool = False) -> ifcopenshell.file: - """Build a minimal IFC4 file containing only an IfcProjectLibrary (no IfcProject). +def _make_library_file(*, with_child: bool = False) -> ifcopenshell.file: + """Build a spec-valid IFC4 library file: IfcProject + IfcProjectLibrary declared to it. - Per IFC4+, a file must contain at least one IfcContext; IfcProjectLibrary is a - valid root on its own. ``with_child=True`` nests a sub-library under the root via - IfcRelNests, mirroring real authored library files. + Per the IFC Project Context concept template, every project data set (library + files included) shall contain exactly one IfcProject, and IfcProjectLibrary + instances are assigned to it via IfcRelDeclares. This matches how every library + file shipped in bonsai/bim/data/libraries is actually authored. ``with_child=True`` + also nests a sub-library under the root via IfcRelNests. """ library_file = ifcopenshell.api.project.create_file(version="IFC4") + project = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProject", name="Demo Project") root = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="RootLib") + ifcopenshell.api.project.assign_declaration(library_file, definitions=[root], relating_context=project) if with_child: child = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="ChildLib") ifcopenshell.api.nest.assign_object(library_file, [child], root) return library_file -class TestLibraryOnlyFile(NewIfc): - def test_get_root_context_returns_project_library_when_no_project(self): - library_file = _make_library_only_file() - assert not library_file.by_type("IfcProject") +class TestLibraryFile(NewIfc): + """Project-library UI code operating on a spec-valid model (IfcProject root). - root = tool.Project.get_root_context(library_file) + A file containing only IfcProjectLibrary and no IfcProject is not valid IFC and + is not supported; see test_parent_libraries_enum_raises_for_a_file_without_a_project. + """ - assert root.is_a("IfcProjectLibrary") - assert root.Name == "RootLib" + def test_parent_libraries_enum_raises_for_a_file_without_a_project(self): + library_file = ifcopenshell.api.project.create_file(version="IFC4") + ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="RootLib") + IfcStore.library_file = library_file + try: + with pytest.raises(IndexError): + ProjectLibraryData.parent_libraries_enum() + finally: + IfcStore.library_file = None - def test_get_parent_library_returns_none_for_root_library(self): - library_file = _make_library_only_file() + def test_get_parent_library_returns_project_for_declared_root_library(self): + library_file = _make_library_file() + project = library_file.by_type("IfcProject")[0] root = library_file.by_type("IfcProjectLibrary")[0] - assert tool.Project.get_parent_library(root) is None + assert tool.Project.get_parent_library(root) == project - def test_get_project_hierarchy_skips_root_library(self): - library_file = _make_library_only_file(with_child=True) + def test_get_parent_library_returns_the_library_for_a_nested_sub_library(self): + library_file = _make_library_file(with_child=True) + root = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "RootLib") + child = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "ChildLib") + + assert tool.Project.get_parent_library(child) == root + + def test_get_parent_library_returns_none_for_an_orphaned_library(self): + library_file = ifcopenshell.api.project.create_file(version="IFC4") + orphan = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="Orphan") + + assert tool.Project.get_parent_library(orphan) is None + + def test_get_project_hierarchy_roots_libraries_under_the_project(self): + library_file = _make_library_file(with_child=True) + project = library_file.by_type("IfcProject")[0] root = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "RootLib") child = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "ChildLib") hierarchy = tool.Project.get_project_hierarchy(library_file) - assert root in hierarchy + assert root in hierarchy[project] assert child in hierarchy[root] - def test_project_library_data_loads_without_crash(self): - IfcStore.library_file = _make_library_only_file() + def test_project_library_data_loads_with_unique_enum_keys(self): + IfcStore.library_file = _make_library_file(with_child=True) try: ProjectLibraryData.is_loaded = False ProjectLibraryData.load() assert ProjectLibraryData.is_loaded enum = ProjectLibraryData.data["parent_libraries_enum"] - assert len(enum) == 1 - assert enum[0][1].startswith("IfcProjectLibrary ") + keys = [entry[0] for entry in enum] + assert len(keys) == len(set(keys)) + assert enum[0][1].startswith("IfcProject ") finally: IfcStore.library_file = None ProjectLibraryData.is_loaded = False - def test_refresh_library_succeeds_on_library_only_file(self): + def test_refresh_library_succeeds(self): import bpy - IfcStore.library_file = _make_library_only_file(with_child=True) + IfcStore.library_file = _make_library_file(with_child=True) try: result = bpy.ops.bim.refresh_library() assert result == {"FINISHED"} @@ -97,13 +124,65 @@ class TestLibraryOnlyFile(NewIfc): IfcStore.library_file = None ProjectLibraryData.is_loaded = False - def test_add_project_library_nests_under_root_when_no_project(self): + def test_edit_project_library_moves_a_project_declared_library_under_another_library(self): import bpy - IfcStore.library_file = _make_library_only_file() + library_file = _make_library_file() + project = library_file.by_type("IfcProject")[0] + root = library_file.by_type("IfcProjectLibrary")[0] + target = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="TargetLib") + ifcopenshell.api.project.assign_declaration(library_file, definitions=[target], relating_context=project) + IfcStore.library_file = library_file + try: + props = tool.Project.get_project_props() + props.selected_project_library = str(root.id()) + props.is_editing_project_library = True + props.parent_library = str(target.id()) + + result = bpy.ops.bim.edit_project_library() + + assert result == {"FINISHED"} + assert tool.Project.get_parent_library(root) == target + assert root.Nests and root.Nests[0].RelatingObject == target + assert not root.HasContext + finally: + if props.is_editing_project_library: + props.is_editing_project_library = False + IfcStore.library_file = None + ProjectLibraryData.is_loaded = False + + def test_edit_project_library_moves_a_nested_library_back_under_the_project(self): + import bpy + + library_file = _make_library_file(with_child=True) + project = library_file.by_type("IfcProject")[0] + child = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "ChildLib") + IfcStore.library_file = library_file + try: + props = tool.Project.get_project_props() + props.selected_project_library = str(child.id()) + props.is_editing_project_library = True + props.parent_library = str(project.id()) + + result = bpy.ops.bim.edit_project_library() + + assert result == {"FINISHED"} + assert tool.Project.get_parent_library(child) == project + assert child.HasContext and child.HasContext[0].RelatingContext == project + assert not child.Nests + finally: + if props.is_editing_project_library: + props.is_editing_project_library = False + IfcStore.library_file = None + ProjectLibraryData.is_loaded = False + + def test_add_project_library_declares_new_library_under_the_project_root(self): + import bpy + + IfcStore.library_file = _make_library_file() library_file = IfcStore.library_file try: - root = library_file.by_type("IfcProjectLibrary")[0] + project = library_file.by_type("IfcProject")[0] before = set(library_file.by_type("IfcProjectLibrary")) result = bpy.ops.bim.add_project_library() @@ -113,9 +192,9 @@ class TestLibraryOnlyFile(NewIfc): new_libraries = after - before assert len(new_libraries) == 1 new_library = next(iter(new_libraries)) - assert new_library.Nests - assert new_library.Nests[0].RelatingObject == root - assert not new_library.HasContext + assert new_library.HasContext + assert new_library.HasContext[0].RelatingContext == project + assert not new_library.Nests finally: IfcStore.library_file = None ProjectLibraryData.is_loaded = False