chore: seed PartIndex positionally from VisorSceneBase.add_dataset's part-node list - #66
Merged
Merged
Conversation
margalva
approved these changes
Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Addresses #18
Context
For a multiblock dataset with multiple parts but no part names defined, on a browser refresh, the per-part state is not preserved. The expected behaviour is that preserving the per-part state on refresh is supported regardless of part names (whether set, unset, unique, or containing duplicates). For multiblock datasets with unique part names, per-part state is preserved.
Investigation traced the issue to
PartIndexbeing seeded from a name-keyed map, so all 66 leaves resolved to one seeded ID and the index retained a single entry. The scene graph and renderer each held one node and one pipeline per leaf, so everyset_part_*trigger for the other 65 resolved to no owning dataset and returned without applying, silently.This PR fixes the PartIndex so the part properties are preserved on a browser refresh.
Python Example
Showing an example illustrating how the issue looks on the Python side. The
many_blocksmultiblock dataset in ourtestsdirectory has 66 parts, none of which have a 'name' attribute set. On 'main' this results in the PartIndex only containing a single part, and when the user refreshes the browser, the per-part state is not preserved for most, as it is not in the server's PartIndex.This PR addresses the issue, resulting in a PartIndex fully populated with the expected 66 parts. The behaviour can be verified in the viewer, as a refresh now preserves the per-part state, relying on the populated PartIndex.
On main:
After this PR:
Description
This pull request refactors how part IDs are assigned and seeded in the VTK dataset handling code, moving from a name-based to a positional (flat index) approach. This change ensures that part IDs match scene-graph node IDs, even when part names are missing or duplicated, which is critical for correct frontend behavior and renderer pipeline mapping. The refactor also simplifies and clarifies the contract between the backend and frontend regarding per-part state management. Extensive new and updated tests are included to pin the new behavior and verify correctness, especially in edge cases like unnamed or duplicate parts.
Core logic and API changes:
PartIndexclass now seeds part IDs positionally from a sequence of scene-graph node IDs (by flat index), not by part name, ensuringpart_id == node_ideven when names are missing or duplicated. The seed is now a sequence of IDs, not a dictionary keyed by name. [1]], [2]], [3]])VisorDatasetandVisorDatasetRegistryconstructors and methods are updated to accept and pass this positional seed (node_ids: Sequence[int] | None) instead of a name-to-id map. [1]], [2]], [3]], [4]])Scene graph and registry cleanup:
Bug fixes and improved error handling:
Testing improvements:
test_part_identity_many_blocks.py) to verify correct part identity assignment in ragged, unnamed multiblock datasets, ensuring one part per leaf and correct ID mapping. ([tests/integration/test_part_identity_many_blocks.pyR1-R98])PartIndexto cover positional seeding, duplicate names, zero IDs, and seed/leaf count mismatches. [1]], [2]])These changes ensure robust, predictable part identity assignment in all cases, improving reliability for both backend operations and frontend visualization.