From 54c00f0306552fdf4e2476f19c595475ad5dedd1 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 4 Jun 2026 10:42:58 +0200 Subject: [PATCH] Fix demo preset crash + scope header refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bpy.ops.bim.new_project(preset='demo') crashed in refresh_bim_tool_headers: the post-commit hook fired for every nested bpy.ops.bim.append_library_element during template loading, and the operator context Blender hands to programmatically-invoked nested operators is stripped of the view-layer attributes the refresh reads. Two changes resolve it. Gate the header refresh in tool.Parametric.refresh_post_commit on operator.bl_idname being one of the EDIT_TYPES finish_op idnames. Only validate-gizmo commits (bim.finish_editing_) now trigger the refresh; demo-loader and other non-edit operators skip it. Querying the registry directly is the canonical signal — string-prefix matching would silently drift if ParametricObject.finish_op changes derivation. Harden tool.Blender.get_active_object so its view_layer fallback also uses getattr; the 150+ callers routed through it now tolerate stripped contexts. _resolve_bim_tool_context applies the same defensive pattern to mode / workspace. Tests: - test_handler_restricted_context covers get_active_object's defensive path and the BimTool-family whitelist (excludes annotation, spatial, structural). - test_handler_forward_compat AST-pins that the gate consults EDIT_TYPES (not a string prefix). - test_wall_header_refresh rewritten — three tests cover the gated-by-registry contract: counter bumps for every commit, finish_op operators refresh headers, others don't. Hotkey-driven in-place edits (S_E / C_E) no longer trigger the refresh — they were caught by the pre-refactor "every commit" design. Left out of scope; the new skip-non-finish test pins this as intentional. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/handler.py | 23 ++-- src/bonsai/bonsai/bim/ifc.py | 2 +- src/bonsai/bonsai/core/tool.py | 2 +- src/bonsai/bonsai/tool/blender.py | 31 ++++-- src/bonsai/bonsai/tool/parametric.py | 23 ++-- .../module/model/test_wall_header_refresh.py | 62 +++++++---- .../test/bim/test_handler_forward_compat.py | 101 ++++++++++-------- .../bim/test_handler_restricted_context.py | 74 +++++++++++++ 8 files changed, 230 insertions(+), 88 deletions(-) create mode 100644 src/bonsai/test/bim/test_handler_restricted_context.py diff --git a/src/bonsai/bonsai/bim/handler.py b/src/bonsai/bonsai/bim/handler.py index a0944929d7..2670739a13 100644 --- a/src/bonsai/bonsai/bim/handler.py +++ b/src/bonsai/bonsai/bim/handler.py @@ -179,27 +179,32 @@ def update_bim_tool_props(): def refresh_bim_tool_headers(): - """Commit-driven refresh of BIM Tool header values (extrusion_depth, - length, x_angle) from the active object's IFC geometry. Must not - write user-intent enums — those encode 'what to build next' and - would silently reset on every IFC commit.""" + """Push the active IFC entity's current header float values + (extrusion_depth, length, x_angle) into ``BIMModelProperties``. + Enum-safe: never writes user-intent enum slots, which are owned by + the selection callback.""" ctx = _resolve_bim_tool_context() if ctx is None: return obj, current_tool, element = ctx - if current_tool.idname == "bim.annotation_tool": + if current_tool.idname not in tool.Blender.get_property_header_tools(): return _read_headers_into_props(obj, element) def _resolve_bim_tool_context(): """Return ``(obj, current_tool, element)`` when an active BIM workspace - tool sees a resolvable IFC element; ``None`` otherwise.""" - obj = bpy.context.active_object + tool sees a resolvable IFC element; ``None`` otherwise. Defensive + against stripped operator contexts — a missing ``active_object`` / + ``mode`` / ``workspace`` short-circuits to ``None`` instead of raising.""" + obj = tool.Blender.get_active_object() if not obj: return None - mode = bpy.context.mode - current_tool = bpy.context.workspace.tools.from_space_view3d_mode(mode) + mode = getattr(bpy.context, "mode", None) + workspace = getattr(bpy.context, "workspace", None) + if mode is None or workspace is None: + return None + current_tool = workspace.tools.from_space_view3d_mode(mode) if not current_tool or current_tool.idname not in tool.Blender.get_list_of_tools(): return None element = tool.Ifc.get_entity(obj) diff --git a/src/bonsai/bonsai/bim/ifc.py b/src/bonsai/bonsai/bim/ifc.py index fd3fe77b78..57e35c5070 100644 --- a/src/bonsai/bonsai/bim/ifc.py +++ b/src/bonsai/bonsai/bim/ifc.py @@ -439,7 +439,7 @@ class IfcStore: BrickStore.end_transaction() IfcStore.end_transaction(operator) bonsai.bim.handler.refresh_ui_data() - tool.Parametric.refresh_post_commit() + tool.Parametric.refresh_post_commit(operator) if method == "MODAL": cls.modal_in_progress = False diff --git a/src/bonsai/bonsai/core/tool.py b/src/bonsai/bonsai/core/tool.py index bd563f0635..c15955824e 100644 --- a/src/bonsai/bonsai/core/tool.py +++ b/src/bonsai/bonsai/core/tool.py @@ -793,7 +793,7 @@ class Profile: @interface class Parametric: def get_geom_generation(cls) -> int: pass - def refresh_post_commit(cls) -> None: pass + def refresh_post_commit(cls, operator) -> None: pass @interface diff --git a/src/bonsai/bonsai/tool/blender.py b/src/bonsai/bonsai/tool/blender.py index 181bef9dd7..b990dd857b 100644 --- a/src/bonsai/bonsai/tool/blender.py +++ b/src/bonsai/bonsai/tool/blender.py @@ -229,15 +229,22 @@ class Blender(bonsai.core.tool.Blender): @classmethod def get_active_object(cls, is_selected: bool = False) -> Union[bpy.types.Object, None]: - """Gets the active object + """Return the active object, or ``None`` when the current context + exposes neither ``active_object`` nor a ``view_layer`` (stripped + operator contexts). :param is_selected: If true, the active object also needs to be selected. """ - if obj := (getattr(bpy.context, "active_object", None) or bpy.context.view_layer.objects.active): - if not is_selected: - return obj - if obj.select_get(): - return obj + obj = getattr(bpy.context, "active_object", None) + if obj is None: + view_layer = getattr(bpy.context, "view_layer", None) + if view_layer is not None: + obj = view_layer.objects.active + if obj is None: + return None + if is_selected and not obj.select_get(): + return None + return obj @classmethod def get_selected_objects(cls, include_active: bool = True) -> set[bpy.types.Object]: @@ -1978,6 +1985,18 @@ class Blender(bonsai.core.tool.Blender): dct = {cls.bl_idname: cls.ifc_element_type for cls in (BimTool.__subclasses__())} return types.MappingProxyType(dct) + @classmethod + @lru_cache + def get_property_header_tools(cls) -> frozenset[str]: + """``BimTool`` plus its parametric subclasses — the workspace + tools whose 3D-view / N-panel header surfaces BIM Tool property + floats (extrusion_depth, length, x_angle). ``AnnotationTool`` + and the non-``BimTool`` workspace tools (spatial / structural / + cad / covering) are excluded by construction.""" + from bonsai.bim.module.model.workspace import BimTool + + return frozenset(cls.bl_idname for cls in (BimTool.__subclasses__() + [BimTool])) + @classmethod def get_object_constraint_props(cls, obj: bpy.types.Object) -> BIMObjectConstraintProperties: return obj.BIMObjectConstraintProperties # pyright: ignore[reportAttributeAccessIssue] diff --git a/src/bonsai/bonsai/tool/parametric.py b/src/bonsai/bonsai/tool/parametric.py index 0d31cfad52..840d21dc42 100644 --- a/src/bonsai/bonsai/tool/parametric.py +++ b/src/bonsai/bonsai/tool/parametric.py @@ -179,17 +179,24 @@ class Parametric(bonsai.core.tool.Parametric): return cls._geom_generation @classmethod - def refresh_post_commit(cls) -> None: - """Post-commit hook for ``tool.Ifc.Operator``: re-syncs scene-level - workspace-tool header fields from current IFC state and bumps the - geometry generation counter so caches keyed off it drop stale - entries on the next draw. Header-only — user-intent enums are - re-targeted on selection change, not here.""" - import bonsai.bim.handler # late import: bim.handler imports tool.* + def refresh_post_commit(cls, operator: bpy.types.Operator) -> None: + """Post-commit hook for ``tool.Ifc.Operator``: bumps the geometry + generation counter so caches keyed off it drop stale entries on + the next draw, and tags viewports for redraw. + Additionally refreshes the BIM Tool header floats for the + validate-gizmo path — operators whose ``bl_idname`` is the + ``finish_op`` of an entry in ``EDIT_TYPES``. That is the only + commit class where selection didn't change but the header + values displayed did. Other operators skip the refresh: they + don't target an active-object header edit, and their commit + context may lack the view-layer attributes the refresh reads.""" cls._geom_generation += 1 - bonsai.bim.handler.refresh_bim_tool_headers() tool.Blender.update_all_viewports() + if operator.bl_idname in {feature.finish_op for feature in cls.EDIT_TYPES}: + import bonsai.bim.handler # late import: bim.handler imports tool.* + + bonsai.bim.handler.refresh_bim_tool_headers() @classmethod def find_by_name(cls, name: str) -> Optional[ParametricObject]: diff --git a/src/bonsai/test/bim/module/model/test_wall_header_refresh.py b/src/bonsai/test/bim/module/model/test_wall_header_refresh.py index 41b8719ec5..cbc6c9dae4 100644 --- a/src/bonsai/test/bim/module/model/test_wall_header_refresh.py +++ b/src/bonsai/test/bim/module/model/test_wall_header_refresh.py @@ -18,21 +18,20 @@ # # This file was generated with the assistance of an AI coding tool. -"""Regression tests for the post-IFC-commit refresh path that re-syncs the -workspace tool header (``BIMModelProperties``) and invalidates the per-wall -gizmo geometry cache. +"""Regression tests for the post-IFC-commit refresh path. -Bug repro before the fix: hotkey operators that edited the active wall in -place (``bpy.ops.bim.hotkey(hotkey="S_E")`` / ``"C_E"``) mutated IFC but never -fired ``active_object_callback`` (no selection change), so the header H/L/A -fields and the gizmo cache both kept showing stale values. ``refresh_ui_data`` -ran, but it never resynced ``BIMModelProperties`` and never invalidated the -per-gizmo-group geometry cache. The fix wires both refreshes through -``tool.Parametric.refresh_post_commit`` and calls it from every -``tool.Ifc.Operator`` epilogue.""" +Two invariants: + +* Every commit bumps ``_geom_generation`` so caches keyed off it drop + stale entries on the next read. +* The BIM Tool header float refresh (``refresh_bim_tool_headers``) fires + only for commits whose operator is a parametric ``finish_op`` from + ``tool.Parametric.EDIT_TYPES`` — the validate-gizmo path. Other + operators skip it; their commit context may lack the view-layer + attributes the refresh reads.""" import types -from unittest.mock import patch +from unittest.mock import MagicMock, patch import bpy import pytest @@ -46,17 +45,42 @@ def _require_real_bpy(): pytest.skip("requires real Blender (bpy is mocked or absent)") -def test_refresh_post_commit_bumps_generation_and_resyncs_header(): - """``refresh_post_commit`` must bump the generation counter and call - ``update_bim_tool_props`` so the workspace tool header re-syncs from IFC.""" - import bonsai.bim.handler as handler +def test_refresh_post_commit_bumps_generation_for_every_operator(): + """The generation counter advances on every commit, regardless of + operator class — it's the cache-invalidation signal for any code + keyed off ``tool.Parametric.get_geom_generation()``.""" from bonsai import tool before = tool.Parametric.get_geom_generation() - with patch.object(handler, "update_bim_tool_props") as mock_resync: - tool.Parametric.refresh_post_commit() + tool.Parametric.refresh_post_commit(MagicMock(bl_idname="bim.append_library_element")) assert tool.Parametric.get_geom_generation() == before + 1 - mock_resync.assert_called_once() + + +def test_refresh_post_commit_refreshes_headers_for_validate_gizmo_operators(): + """Operators whose ``bl_idname`` matches a ``ParametricObject.finish_op`` + in ``EDIT_TYPES`` are the validate-gizmo path: selection didn't + change, but the IFC values backing the BIM Tool header did. The + commit hook must push the new IFC state into the header floats.""" + import bonsai.bim.handler as handler + from bonsai import tool + + finish_op_idname = tool.Parametric.EDIT_TYPES[0].finish_op + with patch.object(handler, "refresh_bim_tool_headers") as mock_refresh: + tool.Parametric.refresh_post_commit(MagicMock(bl_idname=finish_op_idname)) + mock_refresh.assert_called_once() + + +def test_refresh_post_commit_skips_header_refresh_for_non_finish_operators(): + """Other operators must not trigger the header refresh. The refresh + reads ``bpy.context``; for commits invoked from a stripped operator + context (e.g. nested ``bpy.ops`` calls during project setup) this + would raise ``AttributeError`` and break the outer operator chain.""" + import bonsai.bim.handler as handler + from bonsai import tool + + with patch.object(handler, "refresh_bim_tool_headers") as mock_refresh: + tool.Parametric.refresh_post_commit(MagicMock(bl_idname="bim.append_library_element")) + mock_refresh.assert_not_called() def test_geom_generation_invalidates_wall_geom_cache(): diff --git a/src/bonsai/test/bim/test_handler_forward_compat.py b/src/bonsai/test/bim/test_handler_forward_compat.py index 736c5bf3cc..faf4e3c9de 100644 --- a/src/bonsai/test/bim/test_handler_forward_compat.py +++ b/src/bonsai/test/bim/test_handler_forward_compat.py @@ -18,11 +18,12 @@ # # This file was generated with the assistance of an AI coding tool. -"""Forward-compat AST contracts for ``bonsai.bim.handler``. +"""Forward-compat AST contracts for the BIM Tool refresh path. -Pins structural invariants on the post-commit refresh path that no -behavioural test can catch on its own — specifically, that the commit- -driven refresh never writes user-intent enum slots.""" +Pins structural invariants that no behavioural test can catch on its own: +the commit-driven header refresh fires only for the parametric validate- +gizmo operators (``bim.finish_editing_``), never universally — and +the header writer never drifts into user-intent enum writes.""" import ast from pathlib import Path @@ -33,17 +34,13 @@ pytestmark = pytest.mark.model HANDLER_PATH = Path(__file__).parent.parent.parent / "bonsai" / "bim" / "handler.py" +PARAMETRIC_PATH = HANDLER_PATH.parent.parent / "tool" / "parametric.py" -# User-intent enums: encode the user's "what to build next" choice on the -# BIM Tool panel. Writing them from a commit-driven path silently resets -# the user's selection on every IFC mutation — selection-change is the -# only legitimate caller. +# User-intent enums encode the user's "what to build next" choice on the +# BIM Tool panel. The header-only writer must never drift into enum writes; +# user-intent enums are owned by the selection-change path. USER_INTENT_ENUM_ATTRS = frozenset({"ifc_class", "relating_type_id"}) -# Functions that must remain free of user-intent enum writes. Both are -# reachable from ``tool.Parametric.refresh_post_commit``. -ENUM_SAFE_FUNCTIONS = ("refresh_bim_tool_headers", "_read_headers_into_props") - def _function_node(tree: ast.Module, name: str) -> ast.FunctionDef: for node in ast.walk(tree): @@ -57,14 +54,13 @@ def handler_tree() -> ast.Module: return ast.parse(HANDLER_PATH.read_text(encoding="utf-8")) -@pytest.mark.parametrize("fn_name", ENUM_SAFE_FUNCTIONS) -def test_commit_driven_function_does_not_write_user_intent_enums(handler_tree: ast.Module, fn_name: str) -> None: - """The commit-driven refresh path must never assign to user-intent - enum slots (``ifc_class``, ``relating_type_id``). Re-targeting these - from the post-commit hook silently overwrites the user's BIM Tool - panel selection on every IFC mutation; only selection-change callers - may write them.""" - fn = _function_node(handler_tree, fn_name) +def test_read_headers_into_props_writes_only_header_floats(handler_tree: ast.Module) -> None: + """``_read_headers_into_props`` is the header-only writer called from + the selection-driven refresh. It must not assign to user-intent enum + slots (``ifc_class``, ``relating_type_id``); those are the + 'what to build next' choice and have their own targeted writes + earlier in ``update_bim_tool_props``.""" + fn = _function_node(handler_tree, "_read_headers_into_props") offenders = [] for node in ast.walk(fn): if not isinstance(node, ast.Assign): @@ -75,32 +71,49 @@ def test_commit_driven_function_does_not_write_user_intent_enums(handler_tree: a if offenders: msgs = ", ".join(f"{attr} at line {line}" for attr, line in offenders) pytest.fail( - f"{fn_name!r} assigns to user-intent enum slot(s): {msgs}. " - f"Move this assignment to a selection-driven callback." + f"_read_headers_into_props assigns to user-intent enum slot(s): {msgs}. " + f"Header refresh must not re-target the user's BIM Tool panel selection." ) -def test_refresh_post_commit_calls_header_only_entrypoint(handler_tree: ast.Module) -> None: - """``tool.Parametric.refresh_post_commit`` must dispatch into - ``refresh_bim_tool_headers``, not ``update_bim_tool_props``. - The latter re-targets user-intent enums; routing the post-commit - hook through it silently resets the user's BIM Tool selection on - every IFC mutation and crashes on element types absent from the - ``ifc_class`` enum (e.g. ``IfcAnnotation``).""" - parametric_path = HANDLER_PATH.parent.parent / "tool" / "parametric.py" - parametric_tree = ast.parse(parametric_path.read_text(encoding="utf-8")) +def test_refresh_post_commit_gates_header_refresh_on_edit_types_registry() -> None: + """``tool.Parametric.refresh_post_commit`` fires for every IFC + operator commit. Only operators whose ``bl_idname`` matches a + ``ParametricObject.finish_op`` in ``EDIT_TYPES`` (the validate- + gizmo path) must trigger a BIM Tool header refresh — selection + didn't change but the header values did. Other operators must + skip the refresh: they don't target an active-object header edit, + and their commit context may lack the view-layer attributes the + refresh reads. + + The gate must consult the registry, not match a string prefix — + ``EDIT_TYPES`` is the canonical list of parametric features, and + querying it stays correct even if ``ParametricObject.finish_op`` + changes its derivation rule.""" + parametric_tree = ast.parse(PARAMETRIC_PATH.read_text(encoding="utf-8")) fn = _function_node(parametric_tree, "refresh_post_commit") - called_handler_attrs = { - node.func.attr - for node in ast.walk(fn) - if isinstance(node, ast.Call) - and isinstance(node.func, ast.Attribute) - and isinstance(node.func.value, ast.Attribute) - and node.func.value.attr == "handler" - } - assert ( - "refresh_bim_tool_headers" in called_handler_attrs - ), "refresh_post_commit must call bonsai.bim.handler.refresh_bim_tool_headers" - assert "update_bim_tool_props" not in called_handler_attrs, ( - "refresh_post_commit must not call update_bim_tool_props " "(re-targets user-intent enums on every commit)" + found_gated_call = False + for node in ast.walk(fn): + if not isinstance(node, ast.If): + continue + references_registry = any( + isinstance(sub, ast.Attribute) and sub.attr == "EDIT_TYPES" for sub in ast.walk(node.test) + ) + if not references_registry: + continue + for body_node in ast.walk(node): + if ( + isinstance(body_node, ast.Call) + and isinstance(body_node.func, ast.Attribute) + and body_node.func.attr == "refresh_bim_tool_headers" + ): + found_gated_call = True + break + if found_gated_call: + break + assert found_gated_call, ( + "tool.Parametric.refresh_post_commit must gate refresh_bim_tool_headers on an " + "If whose test references EDIT_TYPES (the parametric registry). An ungated call " + "fires the refresh for commits in contexts that strip view-layer attributes; " + "a missing call silently drops the validate-gizmo header refresh." ) diff --git a/src/bonsai/test/bim/test_handler_restricted_context.py b/src/bonsai/test/bim/test_handler_restricted_context.py new file mode 100644 index 0000000000..dac9293275 --- /dev/null +++ b/src/bonsai/test/bim/test_handler_restricted_context.py @@ -0,0 +1,74 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Restricted-context regression test for ``tool.Blender.get_active_object``. + +Some Blender contexts (e.g. the C-side operator context handed to +programmatically-invoked nested ``bpy.ops`` calls) lack the view-layer +attributes a normal UI context exposes. The canonical accessor must +return ``None`` in that case rather than ``AttributeError`` — otherwise +every caller routed through it inherits the same crash class that +originally broke ``bpy.ops.bim.new_project(preset='demo')``.""" + +from unittest.mock import patch + +import pytest + +pytestmark = pytest.mark.model + + +class _RestrictedContext: + """Stand-in for a ``bpy.context`` stripped of view-layer attributes.""" + + def __getattr__(self, name): + raise AttributeError(name) + + +def test_get_active_object_returns_none_in_restricted_context(): + """``tool.Blender.get_active_object`` is the canonical defensive + accessor. Both the primary read (``bpy.context.active_object``) and + the fallback (``bpy.context.view_layer.objects.active``) must + tolerate a stripped context — otherwise the 150+ callers in the + codebase that route through this helper inherit the crash.""" + import bonsai.tool.blender as blender_tool + + with patch.object(blender_tool, "bpy") as bpy_patch: + bpy_patch.context = _RestrictedContext() + assert blender_tool.Blender.get_active_object() is None + + +def test_property_header_tools_whitelists_bim_tool_family_only(): + """``tool.Blender.get_property_header_tools`` gates the validate- + gizmo header refresh. Parametric ``BimTool`` subclasses and the + base ``BimTool`` itself must be included; ``AnnotationTool`` and + workspace tools outside the ``BimTool`` family (spatial, structural, + etc.) must not — they don't surface these header floats.""" + import bonsai.tool as tool_ + + # Ensure the lru_cache picks up subclasses registered by the + # current Blender session (idempotent if already populated). + tool_.Blender.get_property_header_tools.cache_clear() + headers = tool_.Blender.get_property_header_tools() + + assert "bim.bim_tool" in headers, "base BimTool must surface property headers" + assert "bim.wall_tool" in headers, "parametric BimTool subclass must surface property headers" + assert "bim.annotation_tool" not in headers, "AnnotationTool is not a BimTool subclass — no header surface" + assert "bim.spatial_tool" not in headers, "SpatialTool is not BimTool-derived" + assert "bim.structural_tool" not in headers, "StructuralTool is not BimTool-derived"