Fix: cannot load segmentation after export in local mode

[nithin-trenser]

Context

Fix issue : #5540 and related fix in segmentation load for multiframe data : cornerstonejs/cornerstone3D#2727

Segmentation is not loading after export in /local mode.
Fixed jump to segment feature for multiframe data.

Changes & Results

• Loading a DICOM SEG locally in the OSS viewer crashes in OHIFCornerstoneSEGViewport._getReferencedDisplaySetMetadata.
• Cannot destructure property 'SpacingBetweenSlices' of 'a' as it is undefined.
• For the SEG instance, SharedFunctionalGroupsSequence.PixelMeasuresSequence can be an empty array ([]). The current code assumes it has an item and does: PixelMeasures = PixelMeasuresSequence[0] -> undefined then destructures { SpacingBetweenSlices, SliceThickness } from undefined -> throws

Fix : Make destructuring resilient to missing/empty PixelMeasuresSequence by defaulting PixelMeasures to {}.

Added jump to segment feature for multiframe data -> Bug ticket in CS3D

Before fix

Before-fix-segmentation-load.mp4

After Fix

After-Fix-segmentation-load.mp4

Testing

• Go to ohif website. Go to the /local mode.
• Upload any DICOM data.
• Add a segmentation to the image with the brush tool.
• Click on EXPORT DICOM SEG.
• Load the exported DICOM SEG and observe.

Checklist

PR

• My Pull Request title is descriptive, accurate and follows the
  semantic-release format and guidelines.

Code

• My code has been well-documented (function documentation, inline comments,
  etc.)

Public Documentation Updates

• The documentation page has been updated as necessary for any public API
  additions or removals.

Tested Environment

• OS: Ubuntu 24.04
• Node version: 22
• Browser:Chrome 147.0.7727.138

Greptile Summary

This PR fixes a crash when loading a locally-exported DICOM SEG in the OSS viewer and adds imageId propagation to multiframe combined instances to support the jump-to-segment feature.

• OHIFCornerstoneSEGViewport.tsx: guards against an empty or missing PixelMeasuresSequence by defaulting PixelMeasures to {} before destructuring SpacingBetweenSlices/SliceThickness, preventing the runtime crash.
• combineFrameInstance.ts / MetadataProvider.ts: refactors createCombinedValue to cache a shared template and return Object.create(template) on every call, and similarly wraps the NumberOfFrames < 2 path in Object.create, so the imageId assignment added in MetadataProvider._getInstance does not pollute the cached metadata objects in DicomMetadataStore.

Confidence Score: 4/5

Safe to merge with one small follow-up: the bare return instance fallback at the bottom of combineFrameInstance still returns the shared store reference, which the new imageId assignment in MetadataProvider would mutate in a narrow edge case.

The two primary fixes (PixelMeasuresSequence crash and Object.create wrapping in createCombinedValue) are correct and well-targeted. The only remaining concern is the unguarded return instance at line 157 of combineFrameInstance — an unusual path but one that was explicitly part of the mutation-safety refactor and was missed.

platform/core/src/utils/combineFrameInstance.ts — the final fallback return at the bottom of the function

Important Files Changed

Filename	Overview
extensions/cornerstone-dicom-seg/src/viewports/OHIFCornerstoneSEGViewport.tsx	Adds a
platform/core/src/classes/MetadataProvider.ts	Adds imageId to the combined multiframe result; relies on combineFrameInstance returning a fresh object so this write doesn't pollute the shared cache — holds for the main paths but the bare return instance fallback at line 157 of combineFrameInstance is still exposed
platform/core/src/utils/combineFrameInstance.ts	createCombinedValue now caches a template and returns Object.create(template) so callers receive fresh objects; NumberOfFrames<2 path similarly returns Object.create(instance); bare return instance at line 157 remains unguarded

Comments Outside Diff (1)

1. platform/core/src/utils/combineFrameInstance.ts, line 157-161 (link)

   Unprotected bare return instance at the bottom of combineFrameInstance still returns the shared store reference directly. This path is reachable when NumberOfFrames is undefined/NaN (skips the < 2 early return and the > 1 condition), no per-frame/shared functional groups exist, and GridFrameOffsetVector is absent. In that scenario the combined.imageId = imageId assignment in MetadataProvider._getInstance mutates the original object from DicomMetadataStore. Wrapping this in Object.create is consistent with how the NumberOfFrames < 2 path was already fixed.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: platform/core/src/utils/combineFrameInstance.ts
   Line: 157-161
   
   Comment:
   Unprotected bare `return instance` at the bottom of `combineFrameInstance` still returns the shared store reference directly. This path is reachable when `NumberOfFrames` is `undefined`/`NaN` (skips the `< 2` early return and the `> 1` condition), no per-frame/shared functional groups exist, and `GridFrameOffsetVector` is absent. In that scenario the `combined.imageId = imageId` assignment in `MetadataProvider._getInstance` mutates the original object from `DicomMetadataStore`. Wrapping this in `Object.create` is consistent with how the `NumberOfFrames < 2` path was already fixed.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
platform/core/src/utils/combineFrameInstance.ts:157-161
Unprotected bare `return instance` at the bottom of `combineFrameInstance` still returns the shared store reference directly. This path is reachable when `NumberOfFrames` is `undefined`/`NaN` (skips the `< 2` early return and the `> 1` condition), no per-frame/shared functional groups exist, and `GridFrameOffsetVector` is absent. In that scenario the `combined.imageId = imageId` assignment in `MetadataProvider._getInstance` mutates the original object from `DicomMetadataStore`. Wrapping this in `Object.create` is consistent with how the `NumberOfFrames < 2` path was already fixed.

```suggestion
  // Return a fresh proxy so callers (eg. imageId assignment in MetadataProvider)
  // do not mutate the shared `instance` stored in DicomMetadataStore.
  return Object.create(instance);
};

/**
 * Creates a combined instance stored in the parent object which
```

_{Reviews (2): Last reviewed commit: "Merge branch 'master' into fix-segmentat..." | Re-trigger Greptile}

[netlify]

❌ Deploy Preview for ohif-dev failed. Why did it fail? →

Name	Link
🔨 Latest commit	cf98514
🔍 Latest deploy log	https://app.netlify.com/projects/ohif-dev/deploys/6a0eb8cb322b7d00089b4ce9

[greptile-apps]

Mutation of a cached combined instance

createCombinedValue inside combineFrameInstance caches the per-frame object at instance._parentInstance[frameNumber] and returns the same reference on every subsequent call. Setting combined.imageId = imageId permanently stamps that imageId onto the cached object. If the same SOP-instance frame is ever resolved through two different imageIds (e.g., the same DICOM served from two WADO-RS endpoints), the second caller receives a combined instance whose imageId belongs to the first caller.

Additionally, when NumberOfFrames < 2 combineFrameInstance returns the original instance reference unchanged. If frameNumber is somehow truthy for such an instance, combined === instance, and combined.imageId = imageId mutates the shared object stored in DicomMetadataStore directly — not just its per-frame projection.

Prompt To Fix With AI

This is a comment left during a code review.
Path: platform/core/src/classes/MetadataProvider.ts
Line: 63-68

Comment:
**Mutation of a cached combined instance**

`createCombinedValue` inside `combineFrameInstance` caches the per-frame object at `instance._parentInstance[frameNumber]` and returns the same reference on every subsequent call. Setting `combined.imageId = imageId` permanently stamps that `imageId` onto the cached object. If the same SOP-instance frame is ever resolved through two different imageIds (e.g., the same DICOM served from two WADO-RS endpoints), the second caller receives a combined instance whose `imageId` belongs to the first caller.

Additionally, when `NumberOfFrames < 2` `combineFrameInstance` returns the original `instance` reference unchanged. If `frameNumber` is somehow truthy for such an instance, `combined === instance`, and `combined.imageId = imageId` mutates the shared object stored in `DicomMetadataStore` directly — not just its per-frame projection.

How can I resolve this? If you propose a fix, please make it concise.

[Belbin-GK]

Fixed by avoiding mutation of cached/shared metadata objects in combineFrameInstance.

Previously, combined.imageId = imageId modified shared references returned from createCombinedValue, which could leak imageId values across callers. For single-frame instances, it could also mutate the original metadata object stored in DicomMetadataStore.

Updated the implementation to return a cloned object before assigning imageId.