chore: seed PartIndex positionally from VisorSceneBase.add_dataset's part-node list (#66)

Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>

Author: LKasianAnsys

doc/changelog.d/66.maintenance.md

@@ -0,0 +1 @@
+Seed PartIndex positionally from VisorSceneBase.add_dataset's part-node list

src/ansys/visor/viewer/vtk/datasets/part_index.py

@@ -1,18 +1,20 @@
 """
 PartIndex: owns stable runtime part-ID assignment for a dataset.
 
-Built once from a vtkDataObject at add_dataset time.  IDs are seeded from
-the scene-graph node IDs (same JavaScript-safe integer convention) so that
+Built once from a vtkDataObject at add_dataset time.  IDs are seeded
+positionally: the seed is a sequence of scene-graph node IDs (same
+JavaScript-safe integer convention) indexed by flat_index, so that
 part_id == scene-graph node ID -- the contract the frontend relies on to
 apply per-part state (opacity, visibility, color) to the right VTK actor.
-If no seed is provided for a given name, a fresh random ID is generated as
-a fallback.  IDs are session-scoped -- they do not survive a server restart.
+A position with no seed entry takes a fresh random ID as a fallback.
+IDs are session-scoped -- they do not survive a server restart.
 The name <-> id bridge exists so that VisorStateMapper can translate between
 the runtime layer (IDs) and the persistence layer (names) for
 save_state / load_state.
 """
 from __future__ import annotations
 
+from collections.abc import Sequence
 from dataclasses import dataclass
 
 from vtkmodules.vtkCommonDataModel import vtkCompositeDataSet, vtkDataObject
@@ -43,16 +45,20 @@ class PartIndex:
     --------
     - part_id values are assigned once and never change for the lifetime
       of the dataset.
+    - part_id is seeded positionally from the scene-graph node ID at the
+      same flat_index, so part_id == scene-graph node ID.  Part names play
+      no part in runtime identity.
     - Part names must be unique within a dataset for save_state / load_state
       round-trips to be reliable (the same constraint already applies to
       dataset names in the registry).
     - Part IDs are NOT stable across server restarts.
     """
 
-    def __init__(self, data: vtkDataObject, dataset_name: str = "", seed_ids: dict[str, int] | None = None):
+    def __init__(self, data: vtkDataObject, dataset_name: str = "", seed_ids: Sequence[int] | None = None):
         self._parts: dict[int, PartEntry] = {}  # part_id -> entry
         self._name_to_id: dict[str, int] = {}
-        self._seed_ids: dict[str, int] = seed_ids or {}
+        # Positional seed: scene-graph node IDs in leaf order, indexed by flat_index.
+        self._seed_ids: tuple[int, ...] | None = tuple(seed_ids) if seed_ids is not None else None
         self._build(data, dataset_name)
 
     # ------------------------------------------------------------------
@@ -63,6 +69,7 @@ def _build(self, data: vtkDataObject, dataset_name: str) -> None:
         """Build the part IDs and names."""
         if not is_composite_dataset(data):
             self._add_entry(name=dataset_name, flat_index=0)
+            leaf_count = 1
         else:
             composite = data  # type: vtkCompositeDataSet
             it = composite.NewIterator()
@@ -80,16 +87,29 @@ def _build(self, data: vtkDataObject, dataset_name: str) -> None:
                     logger.warning(
                         f"Duplicate part name '{name}' detected in dataset. "
                         f"The name→id map will only retain the last part with this name, "
-                        f"which means save_state/load_state will be unreliable for these parts. "
-                        f"Part IDs remain unique and all runtime operations are unaffected."
+                        f"which means save_state/load_state will be unreliable for these parts."
                     )
                 self._add_entry(name=name, flat_index=flat_idx)
                 flat_idx += 1
                 it.GoToNextItem()
+            leaf_count = flat_idx
+
+        # A seed that does not cover exactly one position per leaf means the seed
+        # source and this traversal disagree; that is a broken invariant.
+        if self._seed_ids is not None and len(self._seed_ids) != leaf_count:
+            logger.error(
+                f"Part ID seed length {len(self._seed_ids)} does not match leaf count {leaf_count} "
+                f"for dataset '{dataset_name}'. Unseeded positions were assigned random part IDs, "
+                f"so part_id == scene-graph node ID does not hold for them."
+            )
 
     def _add_entry(self, name: str, flat_index: int) -> PartEntry:
         """Add a new entry to the part IDs and names."""
-        pid = self._seed_ids.get(name) or get_random_javascript_safe_id()
+        seed = self._seed_ids
+        if seed is not None and 0 <= flat_index < len(seed):
+            pid = seed[flat_index]  # 0 is a valid JavaScript-safe ID; do not use `or`
+        else:
+            pid = get_random_javascript_safe_id()
         entry = PartEntry(part_id=pid, name=name, flat_index=flat_index)
         self._parts[pid] = entry
         self._name_to_id[name] = pid  # last writer wins on collision
@@ -133,7 +153,7 @@ def get_leaf_block(self, part_id: int, data: vtkDataObject) -> vtkDataObject | N
 
     @property
     def name_to_id_map(self) -> dict[str, int]:
-        """Return a dictionary that maps part IDs to ids."""
+        """Return a dictionary that maps part names to part IDs (last writer wins)."""
         return dict(self._name_to_id)
 
     @property

src/ansys/visor/viewer/vtk/datasets/visor_dataset.py

@@ -1,6 +1,7 @@
 """Class representing a Visor dataset."""
 
 import copy
+from collections.abc import Sequence
 from typing import Dict, List
 
 import numpy as np
@@ -29,15 +30,17 @@
 class VisorDataset:
     """Base class for Visor datasets."""
 
-    def __init__(self, id: int, name: str, data: vtkDataObject, part_name_to_id: Dict[str, int], metadata: ExtendedMetadata):
+    def __init__(self, id: int, name: str, data: vtkDataObject, node_ids: Sequence[int] | None,
+                 metadata: ExtendedMetadata):
         self.id: int = id
         self.data: vtkDataObject = data
 
         # PartIndex is the single source of truth for part topology and IDs.
         # It is built directly from the VTK data object — no scene graph required.
-        # seed_ids pins part IDs to the scene-graph node IDs so the frontend can
-        # correlate part_states entries with VTK actor properties.
-        self.part_index: PartIndex = PartIndex(data, name, seed_ids=part_name_to_id)
+        # node_ids is the positional seed (scene-graph node IDs in leaf order); it
+        # pins part IDs to those node IDs so the frontend can correlate part_states
+        # entries with VTK actor properties.
+        self.part_index: PartIndex = PartIndex(data, name, seed_ids=node_ids)
 
         # Get the dataset info from metadata
         self.info: VisorDatasetInfo = self._build_dataset_info(id, name, metadata)

src/ansys/visor/viewer/vtk/datasets/visor_dataset_registry.py

@@ -1,6 +1,7 @@
 """Registry for managing multiple Visor datasets within a scene."""
 
 import re
+from collections.abc import Sequence
 from typing import Dict, List
 
 from ansys.visor.viewer.core.metadata import ExtendedMetadata
@@ -85,7 +86,7 @@ def add(self,
             dataset_id: int,
             dataset_name: str,
             input: VisorDatasetType,
-            part_name_to_id: Dict[str, int],
+            node_ids: Sequence[int] | None,
             metadata: ExtendedMetadata,
             ) -> VisorDataset:
         """
@@ -94,8 +95,8 @@ def add(self,
         self._update_unit(metadata)
 
         # VisorDataset builds its own PartIndex directly from the VTK data object,
-        # seeded with the scene-graph node IDs so part_id == scene-graph node ID.
-        dataset = VisorDataset(dataset_id, dataset_name, input, part_name_to_id, metadata)
+        # seeded positionally with the scene-graph node IDs so part_id == node ID.
+        dataset = VisorDataset(dataset_id, dataset_name, input, node_ids, metadata)
 
         # Store the dataset in the registry
         self.datasets[dataset_id] = dataset

src/ansys/visor/viewer/vtk/scene/base.py

@@ -298,17 +298,22 @@ def add_dataset(self, input: VisorDatasetType, metadata: ExtendedMetadata) -> in
             # Load the dataset into the scene graph
             dataset_id = self._scene_graph.load_dataset(input, dataset_name)
 
-            # Register each leaf's pipeline with the renderer.
+            # Register each leaf's pipeline with the renderer.  The same list object
+            # is the positional seed below, so the PartIndex entries and the
+            # renderer's pipeline keys are the same node IDs by construction.
+            node_ids: list[int] | None = None
             subtree = self._scene_graph.get_descendant_node(dataset_id, include_self=True)
             if subtree is not None:
-                for leaf in subtree.get_descendant_part_nodes(include_self=True):
+                part_nodes = subtree.get_descendant_part_nodes(include_self=True)
+                for leaf in part_nodes:
                     self._renderer.register_node(leaf, leaf.dataset)
 
-            # Seed PartIndex with scene-graph node IDs so that part_id == scene-graph node ID,
-            # which is the contract the frontend relies on to apply per-part state (opacity etc.).
-            part_name_to_id = self._scene_graph.get_part_name_to_id_map(dataset_id)
+                # Seed PartIndex positionally with scene-graph node IDs so that
+                # part_id == scene-graph node ID, which is the contract the frontend
+                # relies on to apply per-part state (opacity etc.).
+                node_ids = [leaf.id for leaf in part_nodes]
 
-            self._dataset_registry.add(dataset_id, dataset_name, input, part_name_to_id, metadata)
+            self._dataset_registry.add(dataset_id, dataset_name, input, node_ids, metadata)
 
             return dataset_id
 

src/ansys/visor/viewer/vtk/scene_graph/base_node.py

@@ -179,15 +179,6 @@ def descendant_part_count(self, include_self=False) -> int:
         """Return the number of descendant part nodes (optionally including self)."""
         return len(self.get_descendant_part_nodes(include_self=include_self))
 
-    def get_descendant_node_name_to_id_map(self) -> dict[str, int]:
-        """
-        Get a mapping of part names to their corresponding node IDs
-        for a specific dataset.
-
-        Returns:
-            dict[str, int]: A dictionary mapping part names to node IDs.
-        """
-        return {node.name: id for id, node in self._get_descendant_node_dict(include_self=True).items() if node.name is not None}
 
     @staticmethod
     def get_node(

src/ansys/visor/viewer/vtk/scene_graph/root_node.py

@@ -117,18 +117,3 @@ def remove_dataset(self, dataset_id: int):
         self._dataset_bounds.remove(dataset_id)
         logger.debug(f"Removed dataset with ID: {dataset_id}")
 
-    def get_part_name_to_id_map(self, dataset_id: int) -> dict[str, int]:
-        """
-        Get a mapping of part names to their corresponding node IDs
-        for a specific dataset.
-
-        Args:
-            dataset_id (int): The ID of the dataset.
-        Returns:
-            dict[str, int]: A dictionary mapping part names to node IDs.
-        """
-        dataset_node = self.get_descendant_node(dataset_id)
-        if not dataset_node:
-            logger.warning(f"Dataset with ID {dataset_id} not found in scene graph.")
-            return {}
-        return dataset_node.get_descendant_node_name_to_id_map()

tests/integration/test_input_data.py

@@ -426,9 +426,9 @@ def test_vtkhdf_multiblock_block_names_are_preserved():
     root_node.load_dataset(dataset, None)
 
     file_node = root_node.children[0]
-    name_to_id = file_node.get_descendant_node_name_to_id_map()
+    node_names = {node.name for node in file_node.get_descendant_nodes(include_self=True)}
     # simple_multiblock.vtkhdf was written with a block named "sphere"
-    assert "sphere" in name_to_id
+    assert "sphere" in node_names
 
 
 def test_vtkhdf_polydata_scene_graph_bounds_are_non_trivial():

tests/integration/test_part_identity_many_blocks.py

@@ -0,0 +1,98 @@
+# python
+# File: `tests/integration/test_part_identity_many_blocks.py`
+"""Part identity over an unnamed, ragged multiblock.
+
+``many_blocks.vtm`` is 66 vtkPolyData leaves nested at four different depths,
+and not one of its 66 ``<DataSet>`` elements carries a ``name`` attribute, so
+every leaf takes the ``"untitled"`` fallback.  Under the old name-keyed seed
+the whole file collapsed to a single part.  These tests pin the positional
+seed: one part per leaf, part_id == scene-graph node ID == renderer pipeline
+key, and flat_index order equal to part-node order on object identity.
+
+All expected counts are hand-written literals, never derived from the VTK
+object under test.
+"""
+import os
+
+import pytest
+from trame.app import get_server
+
+from ansys.visor.viewer.core.metadata import ExtendedMetadata
+from ansys.visor.viewer.vtk.io.file_to_dataset import file_to_dataset
+from ansys.visor.viewer.vtk.scene.local_scene import VisorLocalScene
+
+# many_blocks.vtm holds exactly 66 non-empty leaf blocks.
+MANY_BLOCKS_LEAF_COUNT = 66
+
+
+@pytest.fixture
+def pipeline_instance():
+    """PyTest fixture for pipeline instance."""
+    server = get_server()
+    assert server is not None, "get_server() returned None"
+    scene = VisorLocalScene(server)
+    yield scene
+    try:
+        scene.clear()
+        scene.cleanup_state()
+    except Exception:
+        pass
+
+
+def get_many_blocks_file():
+    """Get the path to the ragged, unnamed multiblock test file."""
+    return os.path.join(os.path.dirname(__file__), "..", "files", "many_blocks", "many_blocks.vtm")
+
+
+@pytest.fixture
+def loaded_many_blocks(pipeline_instance):
+    """Load many_blocks.vtm through the real scene and return the pieces under test."""
+    dataset = file_to_dataset(get_many_blocks_file())
+    metadata = ExtendedMetadata(name="many_blocks", unit="m")
+    dataset_id = pipeline_instance.add_dataset(dataset, metadata)
+
+    subtree = pipeline_instance._scene_graph.get_descendant_node(dataset_id, include_self=True)
+    part_nodes = subtree.get_descendant_part_nodes(include_self=True)
+    part_index = pipeline_instance.datasets[dataset_id].part_index
+
+    return pipeline_instance, dataset, part_nodes, part_index
+
+
+def test_part_index_length_matches_leaf_count(loaded_many_blocks):
+    """One part per non-empty leaf, all IDs distinct.
+
+    Before the positional seed this was 1: all 66 leaves are named "untitled",
+    so the name-keyed seed handed the same ID to every one of them.
+    """
+    _scene, _data, part_nodes, part_index = loaded_many_blocks
+
+    assert len(part_index.part_ids) == MANY_BLOCKS_LEAF_COUNT
+    assert len(set(part_index.part_ids)) == MANY_BLOCKS_LEAF_COUNT
+    assert len(part_nodes) == MANY_BLOCKS_LEAF_COUNT
+
+
+def test_flat_index_order_matches_part_node_order_by_object_identity(loaded_many_blocks):
+    """Composite-iterator order and part-node order refer to the same leaves.
+
+    The discriminator is Python object identity of the leaf ``vtkDataObject``,
+    not bounds: two leaves sharing a bounding box would match by accident.
+    """
+    _scene, data, part_nodes, part_index = loaded_many_blocks
+
+    assert len(part_nodes) == MANY_BLOCKS_LEAF_COUNT
+    for node in part_nodes:
+        assert part_index.get_leaf_block(node.id, data) is node.dataset
+
+
+def test_part_ids_equal_node_ids_equal_pipeline_keys(loaded_many_blocks):
+    """part_id == scene-graph node ID == renderer pipeline key, as three equal sets."""
+    scene, _data, part_nodes, part_index = loaded_many_blocks
+
+    part_ids = set(part_index.part_ids)
+    node_ids = {node.id for node in part_nodes}
+    pipeline_keys = set(scene._renderer._pipelines.keys())
+
+    assert len(part_ids) == MANY_BLOCKS_LEAF_COUNT
+    assert part_ids == node_ids
+    assert part_ids == pipeline_keys
+