Skip to content

feat: Tight boundaries between generic spatial skeleton and catmaid; Use generic arrays over fixed x,y,z ; Update spatial skeleton metadata format#146

Merged
seankmartin merged 19 commits intofeature/edit-modefrom
feature/NA-767
May 7, 2026
Merged

feat: Tight boundaries between generic spatial skeleton and catmaid; Use generic arrays over fixed x,y,z ; Update spatial skeleton metadata format#146
seankmartin merged 19 commits intofeature/edit-modefrom
feature/NA-767

Conversation

@afonsobspinto
Copy link
Copy Markdown
Member

@afonsobspinto afonsobspinto commented May 4, 2026

Closes https://metacell.atlassian.net/browse/NA-731

  • The generic skeleton API defines SpatialSkeletonVector = ArrayLike<number>.
  • Generic node positions use that vector type through SpatiallyIndexedSkeletonNodeBase.position.
  • Bounds and spatial index chunk sizes also use the same vector representation.
  • CATMAID-specific command code converts generic vectors into CATMAID x/y/z

Closes https://metacell.atlassian.net/browse/NA-741

  • The generic source API now exposes fetchNodes(cellIndex: SpatialSkeletonGridCellIndex, ...).
  • SpatialSkeletonGridCellIndex carries a cell vector rather than arbitrary lower/upper bounds.
  • The CATMAID spatial skeleton source implements this by converting the grid cell index and chunk size into CATMAID bounds internally.

Closes https://metacell.atlassian.net/browse/NA-743

  • The generic spatial skeleton metadata now has a spatial array.
  • Each entry contains chunkSize, gridShape, and limit.
  • CATMAID stack metadata is expected to provide metadata.spatial, with each level containing chunk_size and limit. f.e:

{"spatial": [{"limit": 500, "chunk_size": [12058882, 12058882, 12058882]}, {"limit": 500, "chunk_size": [7161485, 7161485, 7161485]}, {"limit": 2500, "chunk_size": [4253037, 4253037, 4253037]}, {"limit": 7000, "chunk_size": [2525778, 2525778, 2525778]}, {"limit": 0, "chunk_size": [1500000, 1500000, 1500000]}], "read_only": false, "cache_provider": "cached_msgpack_grid"}

Closes https://metacell.atlassian.net/browse/NA-767 and https://metacell.atlassian.net/browse/NA-766

  • The generic editable source now requires command (class that implements execute, undo, redo?) factory properties keyed to explicit SpatialSkeletonActions
  • Required editable operations are command factories:
    • addNodesCommand
    • deleteNodesCommand
    • moveNodesCommand
    • splitSkeletonsCommand
    • mergeSkeletonsCommand
  • Optional editable operations are also command factories:
    • insertNodesCommand
    • rerootCommand
    • editNodeDescriptionCommand
    • editNodeTrueEndCommand
    • editNodeRadiusCommand
    • editNodeConfidenceCommand
  • The generic flow asks for the command factory for a given action, creates a command, and runs it through generic command history.
  • CATMAID owns the command implementations, including the state context creation, and the low-level CATMAID API calls.

Closes https://metacell.atlassian.net/browse/NA-725

  • readOnly is part of the generic spatial skeleton source contract.
  • Editable sources are explicitly typed as readOnly: false.
  • CATMAID defaults to read-only.
  • CATMAID stack metadata can explicitly set metadata.read_only: false to enable editing.

…NA-767

# Conflicts:
#	src/layer/segmentation/index.ts
#	src/layer/segmentation/json_keys.ts
#	src/layer/segmentation/spatial_skeleton_serialization.spec.ts
#	src/layer/segmentation/spatial_skeleton_serialization.ts
#	src/skeleton/frontend.ts
@afonsobspinto afonsobspinto marked this pull request as ready for review May 6, 2026 19:48
@afonsobspinto afonsobspinto changed the title Feature/na 767 feat: Tight boundaries between generic spatial skeleton and catmaid; Use generic arrays over fixed x,y,z ; Update spatial skeleton metadata format May 6, 2026
@afonsobspinto afonsobspinto requested a review from seankmartin May 6, 2026 19:52
…NA-767

# Conflicts:
#	src/layer/segmentation/index.ts
#	src/skeleton/backend.ts
#	src/skeleton/frontend.ts
@afonsobspinto afonsobspinto marked this pull request as draft May 7, 2026 00:08
@afonsobspinto
Copy link
Copy Markdown
Member Author

Moved to draft again as I need to re-test it after merging with latest feature/edit-mode

Copy link
Copy Markdown

@seankmartin seankmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally it looks good to me and a much clearer boundary, thank you for the changes here. Most of the comments I've made are minor. The bigint/number issue I didn't spot before now, and it isn't a regression, so I'd personally be fine with us addressing it separately to this PR.

I think the the only other part I'm unsure on is what changed that we can't or at least don't use the code in spatial_chunk_sizing if there is no spatial metadata and instead reject the source

Comment thread docs/user-guide/skeleton_editing.rst Outdated
Comment thread src/datasource/catmaid/frontend.ts Outdated
Comment thread src/skeleton/edit_errors.ts
Comment thread src/skeleton/api.ts
Comment on lines 58 to 59
nodeId: number;
segmentId: number;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work with segmentIDs being bigint? Ids in general, such as NodeIDs and parentNodeId also bigint. I think the problematic part is likely here

  getNodes(
    options: {
      segmentId?: bigint;
      lod?: number;
    } = {},
  ): SpatiallyIndexedSkeletonNode[] {
    void options.lod;
    const normalizedSegmentFilter =
      options.segmentId === undefined
        ? undefined
        : Math.round(Number(options.segmentId));

in the frontend, options.segmentId could be too large to be a number. Not sure if this has any effect on our hash map, but I think that was already designed to work with bigint

Comment thread src/datasource/catmaid/api.ts
Comment thread src/skeleton/spatial_chunk_sizing.ts
Comment thread src/skeleton/spatial_skeleton_manager.ts Outdated
@afonsobspinto afonsobspinto marked this pull request as ready for review May 7, 2026 14:40
@seankmartin seankmartin merged commit bb90d20 into feature/edit-mode May 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants