feat: Add support for labelmap seg images in any supported tsuid (also for compressed bitmap)

[wayfarer3130]

Context

This PR adds integration with the cs3d adapter changes that provide an image loader instead of providing a full binary segmentation object. That results in significantly faster loading and saving of segmentations.

cs3d_ref: fix/use-imageLoader-for-seg

Changes & Results

Testing

Created testdata for all 4 formats we support:
OHIF/viewer-testdata#11
Will upload this data shortly for testing.

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:
• [] Node version:
• [] Browser:

Greptile Summary

This PR integrates the @cornerstonejs/adapters image-loader-based SEG API, replacing the previous ArrayBuffer fetch path with a per-frame imageId approach for faster loading and saving of segmentations. It also adds labelmap SEG SOP class support, configurable store encoding via customizationService, and makes OHIF runtime instance properties non-enumerable to prevent dcmjs from serialising them into DICOM output.

• SEG load path rewritten (getSopClassHandlerModule.ts): _loadSegments now resolves per-frame imageIds through the active data source instead of fetching a raw buffer; createFromDICOMSegBuffer receives an imageId string as its second argument matching the updated adapters API.
• Viewport overlay timing fixed (CornerstoneViewportService.ts): addOverlayRepresentationForDisplaySet now waits for SEGMENTATION_LOADING_COMPLETE before applying the labelmap representation, but the waiting Promise has no rejection or timeout path — a SEG load error leaves it pending indefinitely and leaks the subscriber.
• Non-enumerable runtime properties (combineFrameInstance.ts, both data sources, dicomWriter.ts): imageId, wadoRoot, frameNumber, and _parentInstance are redefined as non-enumerable so they are invisible to dcmjs.datasetToDict, preventing them from being written into stored DICOM files.

Confidence Score: 3/5

The overlay representation setup for SEG viewports can hang indefinitely when a SEG fails to load, blocking the viewport's entire overlay pipeline with no way to recover short of a page reload.

The new async wait-for-SEG pattern in CornerstoneViewportService subscribes to SEGMENTATION_LOADING_COMPLETE but provides no rejection path and no timeout. Because _addOverlayRepresentations sequentially awaits each overlay's addOverlayFn, a single SEG load error stalls it forever — the subscriber also accumulates with each failed attempt. This affects any user who opens a study containing a SEG series that fails to load.

extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts — the addOverlayRepresentationForDisplaySet method needs a rejection path (or timeout) wired to a SEG load-failure event so the returned Promise always settles.

Important Files Changed

Filename	Overview
extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts	New async wait-for-SEG pattern in addOverlayRepresentationForDisplaySet: returns a Promise that never settles when SEG loading fails, blocking _addOverlayRepresentations indefinitely and leaking the subscriber.
extensions/cornerstone-dicom-seg/src/getSopClassHandlerModule.ts	Major rework: replaces ArrayBuffer-based loading with imageId-based approach; adds per-frame imageId resolution helpers, metadata registration, and structured logging.
extensions/default/src/utils/registerNaturalizedDatasetForLocalWadouri.js	New utility: registers a generated DICOM dataset as a blob URL with the wadouri file manager and maps per-frame imageIds in MetadataProvider; logic is sound but the file has excessive blank lines throughout (likely a diff artifact).
extensions/default/src/utils/dicomWriter.ts	New utility: centralises dcmjs dataset serialisation with non-enumerable runtime property handling and transfer-syntax preservation.
platform/core/src/classes/MetadataProvider.ts	Replaces inline URI normalisation with baseImageURIForMetadata, which also strips ?frame= prefix; adds ?frame= frame-info extraction.
platform/core/src/utils/combineFrameInstance.ts	Makes _parentInstance and frameNumber non-enumerable to prevent dcmjs from serialising OHIF runtime fields into DICOM output.
extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts	Null-guards added for missing segmentation state in addSegmentationRepresentation, addOrUpdateSegmentation, and determineViewportAndSegmentationType; imageId resolution now prefers segDisplaySet.referencedImageIds.
extensions/default/src/DicomLocalDataSource/index.js	Switches to non-enumerable instance properties, fixes frameIndex→frameNumber (0-based→1-based), improves getImageIdsForInstance to handle multiframe and missing URLs, and replaces dcmjs.data.datasetToBlob with centralised dicomWriter.
extensions/default/src/DicomWebDataSource/index.ts	Switches instance runtime properties to non-enumerable, preserves transfer syntax from dataset metadata when serialising, and delegates write to centralised dicomWriter.
extensions/cornerstone-dicom-seg/src/viewports/OHIFCornerstoneSEGViewport.tsx	Adds referencedDisplaySet to the stack passed to OHIFCornerstoneViewport so the underlying grayscale series is available when SEG labelmap imageIds are loaded.

Comments Outside Diff (1)

1. extensions/cornerstone-dicom-seg/src/getSopClassHandlerModule.ts, line 216-221 (link)

   Event listener is never removed

   The SEGMENTATION_LOAD_PROGRESS listener is registered every time _loadSegments runs but never cleaned up. Loading N SEG display sets accumulates N listeners, so each subsequent progress tick fires N broadcasts of SEGMENT_LOADING_COMPLETE, causing incorrect progress UI and a growing memory leak.

   Then add eventTarget.removeEventListener(Enums.Events.SEGMENTATION_LOAD_PROGRESS, onProgress); in a finally block (or right after the await).

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/cornerstone-dicom-seg/src/getSopClassHandlerModule.ts
   Line: 216-221
   
   Comment:
   **Event listener is never removed**
   
   The `SEGMENTATION_LOAD_PROGRESS` listener is registered every time `_loadSegments` runs but never cleaned up. Loading N SEG display sets accumulates N listeners, so each subsequent progress tick fires N broadcasts of `SEGMENT_LOADING_COMPLETE`, causing incorrect progress UI and a growing memory leak.
   
   
   
   Then add `eventTarget.removeEventListener(Enums.Events.SEGMENTATION_LOAD_PROGRESS, onProgress);` in a `finally` block (or right after the `await`).
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

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

---

### Issue 1 of 2
extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts:1297-1310
**Promise never settles on SEG load failure**

`addOverlayRepresentationForDisplaySet` returns a `Promise` that resolves only when `SEGMENTATION_LOADING_COMPLETE` fires. If `_loadSegments` throws (e.g., no imageId, adapter error, missing referenced display set), the promise in `_load` rejects but `SEGMENTATION_LOADING_COMPLETE` is never broadcast. The `Promise` returned here never resolves or rejects, so `_addOverlayRepresentations` — which `await`s this — hangs indefinitely, stalling the entire overlay setup for the viewport. The subscriber also leaks because `unsubscribe()` is never called in the error path.

### Issue 2 of 2
extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts:1292-1311
**TOCTOU race: event can fire between guard check and subscribe**

If `SEGMENTATION_LOADING_COMPLETE` fires between the synchronous `getSegmentation(segmentationId)` guard and the `subscribe()` call (possible when the load resolves via a synchronous microtask chain before the outer call stack unwinds), the event is missed and the returned `Promise` hangs forever. Subscribing before the guard check, or using a `getSegmentation` poll-after-subscribe pattern, would close this window.

_{Reviews (10): Last reviewed commit: "fix: Use newer load/seg data set." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[netlify]

✅ Deploy Preview for ohif-dev ready!

Name	Link
🔨 Latest commit	88f2d56
🔍 Latest deploy log	https://app.netlify.com/projects/ohif-dev/deploys/6a0f4e575dabef00071f4ebb
😎 Deploy Preview	https://deploy-preview-5806--ohif-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[greptile-apps]

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[cypress]

Viewers    Run #6304

Run Properties:   Passed #6304  •  88f2d5608b: fix: Use newer load/seg data set.

Project	Viewers
Branch Review	feat/load-seg-images
Run status	Passed #6304
Run duration	02m 24s
Commit	88f2d5608b: fix: Use newer load/seg data set.
Committer	Bill Wallace
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	37
View all changes introduced in this branch ↗︎

[jbocce]

@wayfarer3130 please add details for testing this. Thank you.

[jbocce]

@claude review

[claude]

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, reopen this pull request to trigger a review.

[claude]

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, reopen this pull request to trigger a review.

[greptile-apps]

Promise never settles on SEG load failure

addOverlayRepresentationForDisplaySet returns a Promise that resolves only when SEGMENTATION_LOADING_COMPLETE fires. If _loadSegments throws (e.g., no imageId, adapter error, missing referenced display set), the promise in _load rejects but SEGMENTATION_LOADING_COMPLETE is never broadcast. The Promise returned here never resolves or rejects, so _addOverlayRepresentations — which awaits this — hangs indefinitely, stalling the entire overlay setup for the viewport. The subscriber also leaks because unsubscribe() is never called in the error path.

Prompt To Fix With AI

This is a comment left during a code review.
Path: extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
Line: 1297-1310

Comment:
**Promise never settles on SEG load failure**

`addOverlayRepresentationForDisplaySet` returns a `Promise` that resolves only when `SEGMENTATION_LOADING_COMPLETE` fires. If `_loadSegments` throws (e.g., no imageId, adapter error, missing referenced display set), the promise in `_load` rejects but `SEGMENTATION_LOADING_COMPLETE` is never broadcast. The `Promise` returned here never resolves or rejects, so `_addOverlayRepresentations` — which `await`s this — hangs indefinitely, stalling the entire overlay setup for the viewport. The subscriber also leaks because `unsubscribe()` is never called in the error path.

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

[fedorov]

AI-Assisted Code Review

How this review was generated: This review was produced using Claude Code (model: claude-sonnet-4-6, Anthropic, May 2026). The review process had two passes:

1. Diff review — the full PR diff (2,621 lines, 39 files) was analyzed for correctness, style, and known anti-patterns.
2. Codebase analysis — the branch was checked out locally at /Users/af61/github/Viewers and ~160 targeted file reads and greps were performed across the full monorepo to assess cross-cutting impact on unchanged code.

All findings were verified against the actual source. Sections marked "Cleared" below were flagged in the diff pass but found safe after full-codebase inspection.

---

Overview

The PR replaces the ArrayBuffer-based SEG loading path with a per-frame imageId approach that delegates pixel data loading to the Cornerstone image loader. Key changes:

• New imageId-based API for adaptersSEG.Cornerstone3D.Segmentation.createFromDICOMSegBuffer
• Support for the labelmap SEG SOP class (1.2.840.10008.5.1.4.1.1.66.7) in addition to bitmap
• Configurable store encoding (labelmap vs bitmap, transfer syntax) via customizationService
• OHIF runtime properties (imageId, wadoRoot, frameNumber, etc.) made non-enumerable to prevent dcmjs from writing them into stored DICOM
• Centralized DICOM serialization in a new dicomWriter.ts utility
• dcmjs bumped from 0.49.4 → 0.51.1 across the monorepo

The architectural direction is sound and several pre-existing bugs are fixed. However there is one critical sequencing issue that would likely prevent the primary feature from working.

---

Critical Issues

1. SEGMENTATION_LOADING_COMPLETE fires before segmentation is in state — null-guard silently drops overlay

This is the most significant issue, found via full codebase analysis. In SegmentationService.ts, SEGMENTATION_LOADING_COMPLETE is broadcast (line ~587) before addOrUpdateSegmentation is called (line ~611), which is what actually puts the segmentation into state.

The new subscriber in CornerstoneViewportService.ts fires applyRepresentation() immediately on the event. That calls addSegmentationRepresentation → getSegmentation(segmentationId). Because the segmentation isn't in state yet at that point, the null-guard introduced by this same PR at line 302 triggers — logs a warning and silently returns without adding the overlay representation. The SEG labelmap is parsed and loaded, but never applied to the viewport.

Fix: Move the SEGMENTATION_LOADING_COMPLETE broadcast to after addOrUpdateSegmentation completes, or restructure the subscriber to wait until the segmentation is in state before calling addSegmentationRepresentation.

2. Promise in addOverlayRepresentationForDisplaySet never resolves on SEG load failure

If _loadSegments throws (no imageId, adapter error, missing referenced display set), SEGMENTATION_LOADING_COMPLETE is never fired. The Promise returned by addOverlayRepresentationForDisplaySet never resolves or rejects, so _addOverlayRepresentations stalls indefinitely. In practice this blocks the VIEWPORT_DATA_CHANGED broadcast but not viewport rendering (since viewport.setStack completes before overlay setup is awaited). Still, any downstream update dependent on that broadcast is silently dropped with no way to recover short of a page reload. No error event subscription, timeout, or rejection path is present.

---

Issues Flagged in Diff Pass, Cleared by Codebase Analysis

frameIndex → frameNumber rename: Not a breaking change. MetadataProvider.getUIDsFromImageID always overrides the stored field with the URL-parsed frame value; the stored field name was always ignored.

baseImageURIForMetadata coverage: Correct and consistent. The new function strips both ?frame= and &frame=, fixing an inconsistency in the old code that only stripped &frame=. Keys remain consistent within a session.

getImageIdsForInstance behavior change: Correct fix. The old code always used &frame= even for URLs with no existing query params; appendFrameQueryToImageId correctly chooses ? vs &. No callers depended on the old behavior.

displaySets={[referencedDisplaySet, segDisplaySet]}: Correct fix. OHIFCornerstoneViewport uses only viewportData.data[0].imageIds for the stack; SEG is picked up by _processExtraDisplaySetsForViewport as an overlay. No double rendering.

Non-enumerable frameNumber: Safe. All access patterns use direct property access (destructuring, dot notation) — no code uses Object.keys, JSON.stringify, or spread on combined frame instances expecting frameNumber.

---

Medium Issues

3. tmtv RT export not updated

tmtv/src/utils/dicomRTAnnotationExport/RTStructureSet/dicomRTAnnotationExport.js still calls dcmjs.data.datasetToBlob directly without makeExistingPropertiesNonEnumerable. Any OHIF runtime properties that remain enumerable on RT export instances will be serialized as DICOM tags, potentially corrupting RT output. This is pre-existing, but the PR introduces the fix pattern everywhere else and misses this file.

4. Blob URL memory leak in registerNaturalizedDatasetForLocalWadouri

dicomImageLoader.wadouri.fileManager.add(blob) registers a blob URL that is never revoked. Each report store/download/clipboard operation accumulates a blob in memory for the session lifetime. No fileManager.remove() call exists anywhere in the codebase. For long sessions with many reports this could be meaningful.

5. Duplicate getDatasetTransferSyntaxUID function

Defined privately in both dicomWriter.ts and DicomWebDataSource/index.ts with slightly different fallback logic (the latter falls back to EXPLICIT_VR_LITTLE_ENDIAN; the former returns undefined). The DicomWebDataSource version should import from dicomWriter.ts to avoid future divergence.

6. Dual transferSyntaxUID / transferSyntaxUid keys in segmentationConfig.ts

options.transferSyntaxUID = transferSyntaxUID;
options.transferSyntaxUid = transferSyntaxUID;  // camelCase inconsistency

This is clearly a workaround for an API inconsistency in @cornerstonejs/adapters. Without a comment referencing the upstream issue or PR, future readers will remove one key as an obvious duplicate and silently break one code path.

7. LABELMAP_SEG_SOP_CLASS_UID defined twice

Defined at the top of getSopClassHandlerModule.ts and again in segmentationConfig.ts. The local definition in getSopClassHandlerModule.ts is dead code since getSegmentationParserType is imported from segmentationConfig.ts.

8. registerNaturalizedDatasetForLocalWadouri.js formatting

Every line in this new file is separated by an extra blank line — the entire file is double-spaced. Likely an editor/diff artifact. Makes the file hard to read and should be cleaned up before merge.

---

Minor / Nits

• Verbose console.info in production paths — _logSegImageIds dumps full imageId arrays on every SEG load; registerNaturalizedDatasetForLocalWadouri logs blob byte size and all frame imageIds. Useful for debugging, noisy in production.
• PR checklist not filled out — all checkboxes use []; tested OS/Node/Browser fields are blank.
• @cornerstonejs/adapters 4.22.8 still depends on dcmjs 0.49.4 — per bun.lock, adapters may bundle their own copy alongside OHIF's 0.51.1. Worth verifying the adapters' behavior is compatible with the API surface used in dicomWriter.ts.

---

What's Done Well

• Event listener cleanup — try/finally around SEGMENTATION_LOAD_PROGRESS correctly fixes a pre-existing leak where N SEG loads accumulated N persistent listeners.
• Non-enumerable runtime properties — clean, surgical fix preventing dcmjs from writing OHIF bookkeeping fields into stored DICOM output.
• dicomWriter.ts centralization — single place for dcmjs serialization with consistent options and transfer syntax preservation; a clear improvement over scattered ad-hoc datasetToBlob calls.
• _getSegDataSource fallback logic — correctly prefers dicomlocal for local-scheme URLs, falls back to the active data source.
• appendFrameQueryToImageId — fixes a real bug where &frame= was always used regardless of whether the base URL already had query params.
• Null-guards in addOrUpdateSegmentation and determineViewportAndSegmentationType — reasonable defensive programming; the warn log in each case makes skips visible rather than truly silent.
• SEG viewport stack fix — passing [referencedDisplaySet, segDisplaySet] instead of just [segDisplaySet] is a meaningful and correct fix; the old code left the viewport with only derived labelmap imageIds as the primary stack, which are not displayable as grayscale images.

---

Summary

The critical issue is the event ordering in SegmentationService.ts: SEGMENTATION_LOADING_COMPLETE is broadcast before the segmentation is in state, so the new CornerstoneViewportService subscriber calls addSegmentationRepresentation too early, hits the null-guard introduced by this same PR, and silently exits — meaning the labelmap overlay is never applied. This would be caught only by end-to-end testing; the null-guard makes it invisible as a code-level error.