fix: Use the image loader for dicom-seg to allow compressed images

[wayfarer3130]

Context

The previous segmentation loader claimed labelmap + RLE support but neither actually worked, and it loaded SEG by retrieving the entire DICOM object. This PR makes SEG load through the image loader (per-frame imageIds, so standard back ends' SEG frame retrieve is used and compressed frames are supported), and fixes/extends storage to write labelmap or bitmap with correct RLE.

Faster loading from back ends that support frame retrieve, and significantly smaller storage since RLE compresses segmentation images extremely well.

Changes & Results

SEG load via image loader (adapters)

• createFromDicomSegImageId / createLabelmapsFromSegImageIds build labelmaps from per-frame imageIds + a decode callback (defaults to the image loader), replacing the whole-object decode path. Legacy buffer path retained.
• decodeSegPixelDataFromFrameIds fetches/decodes frames with bounded concurrency (default 16, configurable via concurrency) so per-frame requests overlap; order preserved, fail-fast unchanged.
• New encodePixelData plus perFrameFunctionalGroups normalization; broad Segmentation_4X / generateToolState / generateSegmentation updates.

RLE encoder / decoder fix

• RLE now correctly supports 1 / 8 / 16-bit and emits real runs (previously a count of 1 per pixel), making RLE-stored segmentations small and correct.

Full-instance Part 10 prefetch (single uniform frame registry)

• prefetchPart10Instance registers a fetched Part 10 instance into the @cornerstonejs/metadata NATURALIZED / COMPRESSED_FRAME_DATA framework (parsed by the dcmjs async reader).
• WADO-RS loadImage now consults that registry first (wadors/loadImageFromRegistry.ts) and falls through to the normal /frames/N request when nothing is registered — so WADO-RS and WADO-URI share one registry and one decode path.

Metadata / transfer syntaxes

• TransferSyntaxes enum and isVideoTransferSyntax updates.

Stack-labelmap rendering fix

• viewportReferencesSegmentationImages guards syncStackLabelmapActors so a labelmap is only mounted on viewports that actually display its source images. It resolves the labelmap's referenced source imageIds from each labelmap image's referencedImageId in cache (not the labelmap's own imageIds), which fixes the previously failing stack-segmentation render unit tests while still excluding unrelated series that merely share a frame of reference.

Tests & docs

• New jest tests: encodePixelData, fillSegmentationFrames, perFrameFunctionalGroups, resolveFrameImageIds, segBufferMetadataRoundTrip; migration notes added.

Testing

• Load compressed and uncompressed multiframe SEGs via the image-loader path and confirm correct rendering.
• Store as labelmap and as bitmap, in RLE and uncompressed; round-trip and confirm pixel-accurate restore and reduced size for RLE.
• Render a SEG on a stack viewport (single + multiple segmentations, and with brush editing) — these unit tests pass.
• Confirm a labelmap is not mounted on an unrelated series that only shares a frame of reference.

[sedghi]

putting this as request changes until we figure out the regresssions

[wayfarer3130]

PR #2611 — Review Remediation Plan

PR: Use the image loader for dicom-seg to allow compressed images
Branch: fix/use-imageLoader-for-seg

Every item below was verified against the current source on this branch (5 parallel
verification passes). Line numbers are the actual current locations — several
differ from the original review, and those corrections are noted inline.

Work through items one at a time. Check the box and fill in Status: as you go.

Legend: [ ] todo · [~] in progress · [x] done · [-] won't-fix / documented instead

---

Notes on scope (corrected)

The dev-script --sourceMap --inlineSources changes across the nine package.json
files, the undici override, and the getVolumeViewportScrollInfo NaN fix are NOT
unrelated scope creep. Both sets of changes are required to run tests against the
updated labelmap save/load process and stay in this PR. (The original review's
"split these out" suggestion is dropped.)

Softened / corrected claims from the review, confirmed during verification:

• 16-bit break is on the SEG read-back buffer path (decodeSegFramesFromMultiframe),
  not the encoder. RLE → all-zeros (dcmjs rejects 2-segment frames); uncompressed →
  byte/sample confusion halving frames.
• unpackOneBitPlanar is live (1-bit path) — do NOT delete it. Only decode1,
  unpackOneBitInterleaved, and the interleavedOneBit option are dead.
• perFrameFunctionalGroups.jest pins a real frame-drop / NumberOfFrames-shrink
  mechanism; it doesn't directly assert a PixelData desync, and that drop branch is
  unreachable in the real export path (fillSegmentation throws first).

---

Phase 1 — Critical: silent data corruption (block merge)

Verified 2026-07-08: all of 1a–1e are IMPLEMENTED in the working tree and confirmed
against the current source. ⚠️ They are NOT committed — HEAD
(21b776eb91) still contains the unfixed code (e.g. resolveStackImageId in HEAD still
returns the positional fallback), and a git log -S --all pickaxe finds the fix markers
in no commit on any branch. The Phase 1 work lives in the uncommitted working-tree diff
alongside the rest of the PR. Action: commit these (with the Phase 2 changes) so they
aren't lost.

[x] 1a. Positional frame fallback paints frames onto wrong slices

• Where: labelmapImagesFromBuffer.ts:978-980, :1143-1145, :135-140
• Problem: When findReferenceSourceImageId can't resolve a source imageId, both
  insert paths fall back to positional referencedImageIds[i] / [segFrameIndex].
  This assumes SEG frames are dense and in reference-stack order; sparse/reordered
  frames land on the wrong slice.
• Fix: Skip the frame with a warning — do not guess. Positional fallback is
  only safe for dense, in-order SEGs.
• Status: DONE (working tree, uncommitted). resolveStackImageId now returns
  undefined for an unresolved imageId (no segFrameIndex positional param anywhere in
  the file), with a "Skip, don't guess" comment; findReferenceSourceImageId returns
  undefined instead of guessing; both insert drivers continue (skip the frame) on a
  missing source/stack id. HEAD still has the positional fallback.

[x] 1b. decodeSegFramesFromMultiframe breaks 16-bit both ways

• Where: encodePixelData.ts:145-172 (compare correct 16-bit handling at :351-360)
• Problem: 16-bit RLE routed through dcmjs decodeDcmjsRleRows, which rejects
  2-segment frames and returns all zeros. Uncompressed 16-bit is sliced by pixel
  count on a byte view → each "frame" covers half a real frame (frame count halved).
• Fix: Add a real 16-bit branch: Uint16Array slicing for uncompressed; for RLE
  use the in-repo decodeRLE 16-bit path instead of dcmjs. The encoder in this same
  file emits 16-bit RLE, so the round-trip must actually work.
• Pairs with: Phase 6 16-bit round-trip test.
• Status: DONE (working tree, uncommitted). decodeSegFramesFromMultiframe has a real
  8/16-bit RLE branch (decodeRleMultiByteFrame, per-frame, mirroring the encoder) and the
  uncompressed path reads via getBitmapFramesFromDataset which returns 16-bit as
  Uint16Array by value (no byte/sample halving). Covered by the passing 16-bit RLE and
  uncompressed 16-bit regression tests in encodePixelData.jest.js (so 6b's 16-bit
  round-trip is partially covered at the encode/decode unit level). HEAD lacks this branch.
  ADDENDUM (found by 6b): the uncompressed 16-bit branch of
  getBitmapFramesFromDataset didn't unwrap the naturalized [ArrayBuffer]
  PixelData shape — fixed alongside 6b (see 6b status).

[x] 1c. Chunk subarray / readFromUnpackedChunks byteOffset corruption

• Where: labelmapImagesFromBuffer.ts:193-195 + Segmentation_4X.js:1797-1817
• Problem: chunkPixelData uses subarray (views sharing one buffer), but
  readFromUnpackedChunks indexes chunks[i].buffer from 0, ignoring byteOffset.
  Every chunk after the first reads chunk 0's region. Only triggers when
  maxBytesPerChunk forces >1 chunk.
• Fix: Add chunk.byteOffset to the offset, or .slice() chunks into standalone
  buffers.
• Status: DONE (working tree, uncommitted). readFromUnpackedChunks
  (Segmentation_4X.js) now adds chunk.byteOffset on both the single-chunk fast path
  and the multi-chunk copy loop, with a comment explaining that subarray views share one
  backing buffer. HEAD lacks the offset add.
  ADDENDUM (found by 6b): the same function was still byte-width-blind — it
  returned Uint8Array views with element-based offsets, corrupting 16-bit
  (Uint16Array) chunks. Fixed alongside 6b to honor the chunks' element type
  (see 6b status); 8-bit/packed callers unchanged.

[x] 1d. Exported LABELMAP SEGs lose per-frame plane positions

• Where: generateSegmentation.ts:195-206, seed at :416-418; dcmjs derive()
  resets PerFrameFunctionalGroupsSequence to [].
• Problem: The seed only inserts empty {} groups and dcmjs wipes them, so
  exported LABELMAP frames have no PlanePositionSequence / PlaneOrientationSequence
  / FrameContentSequence. Fails SEG IOD; Slicer / highdicom / dcmqi reject or
  misplace. Only round-trips inside cornerstone (reader matches by SOP UID).
• Fix: Populate per-frame positions from the referenced dataset after
  derivation (mirror dcmjs addSegmentFromLabelmap), not by seeding before derive().
• Pairs with: 1e (same file/derivation) and Phase 6 real-derivation test.
• Status: DONE (working tree, uncommitted). getPlaneSequencesForImage rebuilds each
  frame's PlanePositionSequence / PlaneOrientationSequence from the referenced source
  image's imagePlaneModule after derive(), and fillLabelmapSegmentation threads them
  into the per-frame functional groups via applyPerFrameFunctionalGroups. (Still needs
  the Phase 6a real-derivation round-trip test to lock it end-to-end.) HEAD lacks it.

[x] 1e. Bitmap export crashes for SEGs not starting at slice 0

• Where: generateSegmentation.ts:264-287 + Segmentation_4X.js:236-247
• Problem: Derivation built from filteredImages (empty frames removed) but
  referencedFrameNumbers uses original labelmap indices → dcmjs indexes the shortened
  PerFrameFunctionalGroupsSequence / ReferencedInstanceSequence out of bounds
  (TypeError; e.g. lesion on slices 10–12). Regression vs pre-PR.
• Fix: Build the derivation from full images, or remap referencedFrameNumbers
  into filtered-index space consistently.
• Status: DONE (working tree, uncommitted). The BITMAP export dispatch builds the
  derivation from the full, unfiltered images (_createMultiframeSegmentationFromReferencedImages(images, …))
  and passes full images to fillSegmentation, with a comment explaining dcmjs indexes
  source geometry by original frame index. (NOTE: the LABELMAP path still filters to
  non-empty frames — safe there because 2f's shared collectNonEmptyFrameIndices keeps
  fill and dispatch aligned. Confirm the 6a round-trip covers the slice-10 lesion case.)
  HEAD lacks the full-images derivation.

---

Phase 2 — Major: back-compat breaks (fix, or make opt-in + document as 5.x breaking)

Policy decision needed first: restore old behavior vs keep new + document as
intentional 5.x breaking change. Decide per item before coding.

[ ] 2a. Default export SOP class silently → Label Map (66.7)

• Where: generateSegmentation.ts:251
• Problem: Default is now 1.2.840.10008.5.1.4.1.1.66.7 (Label Map, added to DICOM
  2024); most PACS/viewers reject it. BINARY (66.4) is now opt-in.
• Fix: Make the default SOP class BINARY, and add a configuration to OHIF datasources to allow choosing the sop class and transfer syntax for use, defaulting to Labelmap/RLE
• Status: DONE.
  • Adapter default → BINARY: generateSegmentation (cs3d) now defaults
    sopClassUID to BINARY (66.4) instead of Label Map (66.7) when the caller passes
    no option, so external callers get the broadly-compatible class. JSDoc documents it.
  • OHIF datasource config: already existed (segmentation.store.defaultMode =
    labelmap|bitmap, segmentation.store.transferSyntaxUID) via customizationService
    in extensions/cornerstone-dicom-seg/src/utils/segmentationConfig.ts. Mode already
    defaulted to labelmap; added the missing piece — getSegmentationSaveOptions now
    defaults transferSyntaxUID to RLE Lossless (1.2.840.10008.1.2.5) when the
    customization is unset. Net: OHIF defaults to Label Map + RLE, both overridable
    per datasource; the adapter's intrinsic BINARY default only affects non-OHIF callers.
  • No OHIF tests existed for this helper; adapter suite unaffected (no test relied on the
    old LABELMAP default). NOTE: this changes OHIF's default SEG export encoding from
    Explicit-VR-LE to RLE — intended per the plan; flag for any E2E fixtures that pinned
    the uncompressed output.

[ ] 2b. rleEncode removed/ignored; default TS → RLE Lossless; unknown syntaxes throw

• Where: Segmentation_4X.js:46-49 (defaults), :353-358 (throw)
• Problem: rleEncode option no longer read or warned about; default transfer
  syntax changed to RLE Lossless; unknown syntaxes now throw.
• Fix: Document rleEncode as being obsolete - it didn't correctly work previously, so just documenting is sufficient.
• Status: DONE (docs). Expanded the fillSegmentation JSDoc (Segmentation_4X.js) to
  document the supported transferSyntaxUid values (default RLE Lossless; EVRLE also
  supported; anything else throws) and to mark rleEncode obsolete/ignored, noting it
  never produced a valid RLE SEG and RLE is now selected via transferSyntaxUid.

[ ] 2c. generateToolState (default cs3dVersion=4) return shape changed

• Where: Cornerstone3D/Segmentation/generateToolState.ts:14-41
  (NOT the ParametricMap file — review misattributed this)
• Problem: labelmapBufferArray / segmentsOnFrameArray gone (now labelMapImages);
  skipOverlapping accepted but silently dropped on the default cs3dVersion===4 path
  (only forwarded on the legacy !==4 branch).
• Fix: Update documentation to explain return type. Document that skipOverlapping is no longer supported for cs3dVersion===4
  as it was only used by legacy code before overlapping returns were supported in OHIF.
• Status: DONE (docs). Expanded the generateToolState JSDoc (generateToolState.ts):
  documented the new v4 return shape (labelMapImages / segMetadata / segmentsOnFrame
  / centroids / overlappingSegments; the old labelmapBufferArray /
  segmentsOnFrameArray are gone) and that skipOverlapping is ignored on the default
  cs3dVersion === 4 path (still forwarded on the legacy branch), with the rationale.

[ ] 2d. FRACTIONAL SEGs now throw unconditionally

• Where: labelmapImagesFromBuffer.ts:713-715
• Problem: 4.x accepted pseudo-binary fractional objects; now throws.
• Fix: keep the FRACTIONAL branch but replicate the 4.x onlyMaxAndZero test on the decoded frames — if all values are 0/max, proceed down the binary path (values are truthy, so the existing fill logic already maps them to segment indices); only throw for genuinely fractional data. Add a unit test.
• Status: DONE. Moved the FRACTIONAL check to after the frames are decoded/unpacked
  (was an unconditional throw before decode). A new isPseudoBinaryFractional(pixelData, MaximumFractionalValue) helper replicates the 4.x onlyMaxAndZero test; pseudo-binary
  objects (pixels only 0/max) now flow down the existing binary insert path (segment index
  comes from the functional groups; non-zero magnitude is irrelevant), and only genuinely
  fractional data throws. Helper exported + unit-tested in
  test/fractionalPseudoBinary.jest.js (0/max accepted incl. non-255 & string max; missing
  max and intermediate values rejected).

[-] 2e. createLabelmapsFromBufferInternal 2nd param ArrayBuffer → imageId

• Where: labelmapImagesFromBuffer.ts:1607-1620
• Problem: Same name, 2nd positional arg now segImageId (string). Deep importers
  passing an ArrayBuffer break silently.
• Status: WON'T-FIX. Confirmed there is no consumer: zero references in OHIF (current
  branch and origin/master), zero in the cs3d examples, and not re-exported from any public
  entrypoint (only its own module). The sole in-repo caller on origin/main was the sibling
  generateToolState.ts, which this branch rewired to other functions. Risk is purely a
  theoretical deep dist-path importer, so skipped per the plan.

[ ] 2f. Only inputLabelmaps3D[0] exported; labels >255 wrap mod 256; forced 8-bit

• Where: generateSegmentation.ts:90, :75-77, :140
• Fix: Export all labelmaps3D; support >255 (BitsAllocated=16) instead of truncating. Add a unit test.
• Status: DONE. fillLabelmapSegmentation no longer takes only inputLabelmaps3D[0]:
  it unions the non-empty frames and segment metadata across ALL input labelmap3D objects
  and overlays them per frame (later labelmap wins on a voxel clash, warned once — a
  single-valued LABELMAP can't hold overlaps). Bit depth is now chosen from the actual
  max segment value: labels > 255 export as 16-bit (BitsAllocated/BitsStored/HighBit =
  16/16/15, Uint16Array frames, encodeFramesToTransferSyntax bitsAllocated 16) instead
  of being truncated mod 256 by a forced Uint8Array. The export dispatch's referenced-image
  filtering was switched to the same shared collectNonEmptyFrameIndices union so derived
  images stay aligned with the frames written. NOTE: OHIF passes a single labelmap3D, so its
  behavior is unchanged (single-labelmap is the identity case). Dead combinedPixelData
  allocation + now-unused normalizeFramePixelData removed (overlaps 5d). Helpers exported
  and unit-tested in test/labelmapSegmentationExport.jest.js (union frames/metadata, >255
  detection, clash precedence). A full end-to-end BitsAllocated===16 dataset assertion is
  left to the real-derivation round-trip (6a/6b).

[ ] 2g. packBits runs cross row boundaries

• Where: encodePixelData.ts:365-412
• Problem: PS3.5 Annex G requires per-row encoding; row-strict decoders/validators
  reject cross-row runs.
• Fix: Reset run/literal scanning at each row boundary.
• Pairs with: Phase 6 packBits boundary tests. Include this in the phase 2 fix.
• Status: DONE. Refactored packBits into a range encoder (packBitsRange) plus a
  row loop: when a rowLength (bytes per row) is supplied and divides the segment, each
  row is encoded independently so no replicate/literal run crosses a row boundary. Threaded
  columns through encodeFramesToTransferSyntax → encodeFrameToRle → packBits, and
  passed it from all three call sites (labelmap export, bitmap dispatch, fillSegmentation
  RLE path). Row length is columns for 8/16-bit byte planes; for bit-packed 1-bit planes
  it's only applied when columns % 8 === 0 (otherwise rows aren't byte-aligned → falls
  back to whole-segment, safe). columns is optional, so existing callers/tests are
  unchanged. Encoding stays lossless — decoded output is identical regardless of run resets.
  Tests in test/packBits.jest.js cover the 6f cases: replicate run of exactly 128
  (control 129) and 129 (128+literal), literal run of 128 (control 127), no -128 no-op
  byte, per-row split vs whole-segment, non-dividing rowLength fallback, row-by-row
  round-trip, and even-length segment padding. This closes 6f.

[ ] 2h. ReferencedSegmentNumber=1 hardcoded per LABELMAP frame

• Where: generateSegmentation.ts:198-199
• Problem: For LABELMAP the SegmentIdentificationSequence macro must be absent; and
  1 is wrong for frames carrying labels 2+.
• Fix: Omit the macro on the LABELMAP path.
• Status: DONE. applyPerFrameFunctionalGroups now emits
  SegmentIdentificationSequence only when a frame carries a referencedSegmentNumber
  (BINARY, one-segment-per-frame), and strips any inherited one otherwise. The LABELMAP
  export path (generateSegmentation.ts) no longer passes referencedSegmentNumber: 1,
  so the macro is absent for LABELMAP SEGs (correct per the standard; the fixed 1 was
  wrong for labels >= 2). BINARY export via fillSegmentation still passes the real
  segmentIndex, so its behavior is unchanged. Added two perFrameFunctionalGroups.jest.js
  cases (macro omitted with no segment number; inherited macro stripped, other fields kept).

[x] 2i. Multiframe wadouri/dicomfile lose ReferencedFrameNumber

• Where: labelmapImagesFromBuffer.ts:758-769 + Segmentation_4X.js:1547-1555
• Problem: sopUIDImageIdIndexMap stores frame-stripped ids, so
  replace(/frame=\d+/) is a no-op → every SEG frame maps to frame 1, overlapping
  segments dropped. (wadors unaffected — re-appends /frames/N.)
• Fix: Store frame-qualified ids, or carry the frame number separately.
• Pairs with: Phase 6 load-side ReferencedFrameNumber test.
• Status: DONE. Root cause confirmed: the shared consumer
  getImageIdOfSourceImageBySourceImageSequence (Segmentation_4X.js) rewrites the
  frame number by regex and expects a frame-qualified string; the new-path builder
  fed it a stripFrameQualifiersFromImageId() base, so ?frame=/&frame= schemes
  had nothing to replace. The map is also threaded through the shared
  findReferenceSourceImageId, so changing the map's value shape (e.g. a
  frame-indexed array) was rejected as it would break the legacy path and touch
  shared code. Chosen fix (minimal, matches legacy builder behavior): the new-path
  builder now stores the first frame's frame-qualified imageId instead of a
  stripped base, so the existing rewrite reconstructs each frame correctly for all
  schemes.
  • Extracted the inline reduce into an exported buildSopUIDImageIdIndexMap()
    helper in labelmapImagesFromBuffer.ts (documents the frame-qualified contract;
    also adds a ?? {} guard on the metadata lookup).
  • No change to Segmentation_4X.js / any shared helper.
  • Test: packages/adapters/test/sourceImageFrameMapping.jest.js — asserts every
    frame of a multiframe wadouri and dicomfile source resolves to its own
    source frame (not frame 1), with wadors as a passing control. Verified red
    (wadouri/dicomfile collapsed to the stripped base) → green after the fix;
    related adapter seg suites still pass.

[ ] 2j. BINARY processChunk uses return not continue → promise hangs forever

• Where: labelmapImagesFromBuffer.ts:982-1011 (labelmap path correctly uses
  continue at :1150)
• Problem: Three early exits return out of the chunk driver without resolving or
  scheduling the next chunk → load promise never settles on missing stack image /
  instance metadata.
• Fix: Use continue (skip the frame), matching the labelmap path.
• Status: NO CHANGE NEEDED (already fixed on this branch). The BINARY processChunk
  now uses continue at all three early exits (missing source imageId, missing stack
  entry, missing instance metadata). The only return in the driver is a legitimate
  return resolve(insertOverlappingPixelDataPlanar(...)) that switches to overlapping
  handling and settles the promise. Verified by inspection of the current source.

[ ] 2k. Unrecognized scheme / wadors w/o /frames/ silently repeats base imageId

• Where: labelmapImagesFromBuffer.ts:403-405 (review said :411)
• Problem: Returns N copies of the same base imageId → every slice gets frame 1's
  pixels.
• Fix: Throw unless the caller supplies frameImageIds / getFrameImageId.
• Pairs with: Phase 6 test fix (resolveFrameImageIds.jest).
• Status: DONE. resolveFrameImageIds now throws for a multiframe SEG whose scheme
  is neither WADO-RS nor WADO-URI (was returning N copies of the base id → frame 1 on
  every slice). Single-frame unknown schemes still return [segImageId]. Retargeted the
  resolveFrameImageIds.jest.js test that pinned the repeated-base fallback to assert the
  throw, and added a single-frame passthrough case (covers the 6h retarget for this file).

---

Phase 3 — Major: prefetch registry memory lifecycle

[x] 3a. No release API; unbounded static NATURALIZED map

• Where: prefetchPart10Instance.ts:28-33 → addDicomPart10Instance → static
  uncapped Map (cacheData.ts:22)
• Problem: Full compressed instances live outside the size-capped image cache,
  doubled by decoded copies. Memory grows monotonically across studies.
• Status: EVALUATED — "no release API" is INVALID; the OHIF-side wiring gap is REAL.
  Verified two release mechanisms exist and are exported from @cornerstonejs/metadata
  (utilities barrel): clearCacheData() (clear-all, cacheData.ts:87-92, wipes every
  typed map + FRAME_IMAGE_IDS/BASE_IMAGE_ID) and clearTypedCacheData(type, query?)
  (per-instance/per-type, cacheData.ts:94-123; for NATURALIZED also cascades to the
  registered secondary types — INSTANCE, URI_MODULE, IMAGE_PLANE, FRAME_MODULE,
  GENERAL_IMAGE — and the per-query id maps). Weak-holding correctly rejected: the
  registry holds the compressed source-of-truth frames and must survive image-cache
  eviction mid-load; lifecycle release (mode exit / clear cached data) is the right model.
  Remaining action (OHIF repo, not this lib): OHIF calls
  dicomImageLoader.prefetchPart10Instance (extensions/default/src/DicomWebDataSource/index.ts:379)
  but never calls either clear — mode-exit teardown (extensions/cornerstone/src/index.tsx
onModeExit; CornerstoneViewportService.destroy → cache.purgeCache()) purges only
  the cornerstone image cache, so registry memory does persist across studies today.
  Fix: call clearCacheData() in the cornerstone extension's onModeExit (the extension
  already depends on @cornerstonejs/metadata), alongside the pool clears; optionally
  re-export a clear wrapper from dicomImageLoader for symmetry with
  prefetchPart10Instance.
  APPLIED (OHIF repo, not this PR's diff): extensions/cornerstone/src/index.tsx
onModeExit now calls csMetadataUtilities.clearCacheData() (imported from
  @cornerstonejs/metadata) immediately after the image load/retrieval pool clears,
  releasing prefetched Part 10 instances and all derived typed-metadata entries on
  mode exit. No lib change needed.

[-] 3b. Registry lookup on every WADO-RS load; permanent BASE_IMAGE_ID entry per frame

• Where: wadors/loadImage.ts:133-142
• Problem: Lookup runs on every load and permanently inserts one BASE_IMAGE_ID entry
  per frame imageId into a never-evicted map — a new unbounded cost for all wadors users,
  prefetch or not.
• Fix: Gate the lookup so non-prefetch users pay nothing (skip when registry empty /
  behind a flag).
• Status: EVALUATED — mechanics CONFIRMED, severity DOWNGRADED to minor; largely
  resolved by the 3a clear-all wiring. Verified chain: every wadors loadImage →
  loadImageFromCompressedFrameRegistry → getTyped(COMPRESSED_FRAME_DATA) →
  naturalLookup (dataLookup.ts:32) → NATURALIZED chain → baseImageIdQueryFilter
  (naturalizedHandlers.ts:100) → getTyped(BASE_IMAGE_ID), whose cache provider
  (registerCacheProviders, cacheData.ts:330) permanently caches one
  frameImageId → baseImageId entry. BUT: NATURALIZED itself does NOT accumulate on a
  miss (fromAsyncLookup returns undefined without caching, cacheData.ts:139-141)
  and COMPRESSED_FRAME_DATA has no cache type, so the per-frame cost is one
  string→string pair (base-id normalization result), not pixel data — a few hundred
  bytes per distinct frame imageId, single-digit MB even for very large studies, freed
  by the same mode-exit clearCacheData() from 3a. Per-load CPU is a cached map hit +
  short provider chain — negligible vs network/decode, and the lookup is already
  try/catch-guarded. Verdict: don't block merge; the "gate when registry empty"
  optimization is optional and would need a small new API (no exposed
  "type-map is empty" check today, e.g. hasAnyCacheData(type)).

[x] 3c. createImage failure is terminal, loses server-transcoding fallback

• Where: loadImageFromRegistry.ts:86-94
• Problem: No .catch; because the registry path returns early, the /frames/N
transfer-syntax=* negotiation never runs. A frame the worker can't decode rejects
  with no re-request.
• Fix: On decode failure, fall back to the network /frames/N path.
• Status: EVALUATED — VALID, should fix. Confirmed in current source:
  loadImageFromRegistry.ts:86-94 chains createImage(...).then(...) with no
  .catch, and wadors/loadImage.ts:133-142 returns early when the registry path
  yields a promise, so a worker decode failure rejects the frame load and the network
  /frames/N request (whose transfer-syntax=* accept header lets the server
  transcode) never runs — a recoverable case (codec the worker can't decode, corrupt
  registered frame) becomes terminal, only for prefetched instances. Fix shape:
  extract the network branch of loadImage into a function and, on registry-decode
  rejection, console.warn + fall through to it (optionally drop/flag the failing
  instance in the registry so subsequent frames skip the doomed decode). Note the
  registry path returns a bare {promise, cancelFn: undefined} rather than the
  ProgressiveIterator, so the fallback must resolve the same promise the caller
  already holds — chain via .catch(() => networkLoad().promise), don't re-call
  loadImage.
  APPLIED: wadors/loadImage.ts — the network branch is extracted into
  loadImageFromNetwork() (body unchanged); the registry path now chains a
  .catch() that console.warns and resolves the caller's original promise with
  loadImageFromNetwork(imageId, options).promise, restoring the server-transcoding
  fallback for undecodable/corrupt registered frames. Package tsc --noEmit passes.
  No unit test added (package has only karma browser tests; flag for 6c if a jest
  harness lands there). The optional "flag the failing instance so later frames skip
  the doomed decode" was skipped — each frame independently falls back, which is
  correct, just re-attempts one decode per frame.

---

Phase 4 — Major: viewportReferencesSegmentationImages gate

[x] 4a. Unguarded against getImageIds() throw

• Where: viewportReferencesSegmentationImages.ts:74-75 (review said :80)
• Problem: Optional chaining guards method existence, not the throw.
  BaseVolumeViewport.getImageIds throws when there's no actor/volume yet →
  add-before-setVolumes flows now throw.
• Fix: Wrap in try/catch; treat a throw as "no ids known" (permissive).
• Status: EVALUATED — CONFIRMED valid. BaseVolumeViewport.getImageIds
  (BaseVolumeViewport.ts:2439-2456) throws in TWO cases: no volume actor yet
  (_getApplicableVolumeActor returns undefined → "No actor found") and volume
  evicted/missing from cache ("imageVolume … does not exist in cache"). The
  optional chaining at viewportReferencesSegmentationImages.ts:74-75 only guards
  method existence, so a labelmap add before setVolumes (or during teardown)
  now throws out of internalAddSegmentationRepresentation — a regression vs the
  pre-gate API. The stack path is safe (StackViewport.getImageIds just returns
  this.imageIds, StackViewport.ts:3524). Preferred fix: implement via the 4b
  reshape below (getAllVolumeIds() + cache.getVolume(...)?.imageIds never
  touches actors, so it can't throw); keep a defensive try/catch → permissive
  true as backstop.
  APPLIED (with 4b): new getViewportImageIds() helper in
  viewportReferencesSegmentationImages.ts prefers getAllVolumeIds() +
  volume-cache resolution (never touches actors) and wraps everything in
  try/catch → [] → permissive true. Pinned by the spec's "throw safety" and
  "before any volume is set" cases (6g).

[x] 4b. Fusion viewports check only the first/default volume

• Where: viewportReferencesSegmentationImages.ts:75
• Problem: getImageIds() called with no volumeId → only default volume's ids.
  On CT+PT, PT-derived labelmaps are silently suppressed.
• Fix: Iterate all volume actors' imageIds.
• Status: EVALUATED — CONFIRMED valid and user-visible. getImageIds() with no
  volumeId resolves _getApplicableVolumeActor(undefined) → actorEntries[0]
  (BaseVolumeViewport.ts:1700), i.e. the first/default volume only. On CT+PT
  fusion the default actor is CT, so a PT-derived labelmap has zero imageId
  intersection → gate returns false → representation silently dropped (compounded
  by 4c's silent return). Fix shape: for volume viewports, union imageIds across
  ALL volumes via getAllVolumeIds() (BaseVolumeViewport.ts:2549, backed by
  this.volumeIds which is populated in setVolumes/addVolumes and is simply
  empty → [] → permissive before any volume is set) + cache.getVolume(id)?.imageIds.
  This also resolves 4a structurally (no actor access → nothing to throw).
  APPLIED: volume viewports now union imageIds across all volumes via
  getAllVolumeIds() + cache.getVolume(id)?.imageIds (missing/evicted volume →
  no ids → permissive); the no-arg getImageIds() is only used for stack
  viewports. Fusion cases pinned by the 6g spec (PT-derived labelmap on CT+PT
  matches; fully unrelated labelmap suppressed).

[x] 4c. One-shot silent drop of the representation

• Where: internalAddSegmentationRepresentation.ts:32-40
• Problem: Bare return; drops the representation with no warn / event / return
  value, evaluated once at add time; no retry when the stack later changes.
• Fix: Add a console.warn when suppressing; reconsider a retry hook.
• Status: EVALUATED — CONFIRMED. Bare return; at
  internalAddSegmentationRepresentation.ts:39; no warn, no event, no return
  value, and addSegmentationRepresentations discards the mapped results
  (addSegmentationRepresentationsToViewport.ts:12-14), so no caller can detect
  the drop. Because the representation is never added to the state manager, no
  later sync pass can revive it — genuinely one-shot at the cs3d level. (Practical
  impact is limited in OHIF, which re-adds representations when viewport data
  changes; external cs3d consumers get silent nothing.) Recommend: add the
  console.warn (trivial, do with 4a/4b); the retry hook is a real design change —
  defer it, the warn plus 4a/4b's more-permissive gate covers the realistic cases.
  APPLIED: the suppression branch in internalAddSegmentationRepresentation.ts
  now console.warns with the segmentationId and viewportId before returning.
  Retry hook deferred as planned.

[x] 4e. Gate breaks OHIF same-FOR overlay flows (downstream CI red, found 2026-07-09)

• Where: viewportReferencesSegmentationImages.ts (imageId-intersection check) via
  internalAddSegmentationRepresentation.ts:32-43
• Problem: The "OHIF downstream tests against this PR" check on run
  29017653310 (HEAD 3e69b44c9a vs OHIF master) fails 3 Playwright e2e tests, all
  one root cause: the gate requires the viewport to display one of the labelmap's
  referenced imageIds, but OHIF's shipped flows render labelmaps on same-Frame-of-
  Reference viewports showing a DIFFERENT series (volume viewports resample
  geometrically):
  1. DataOverlayMenuFrameOfReference.spec.ts:33 — seg added to FOR1 viewport A must
     sync to same-FOR viewport B (different display set); gate suppresses on B.
  2. SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts:17 — data-overlay of
     a SEG onto a display set it does NOT reference; overlay missing from screenshots.
  3. TMTVCSVReport.spec.ts:4 — TMTV creates the labelmap from PT
     (extensions/tmtv/src/commandsModule.ts:146); representation never reaches the
     CT viewports (no PT imageId intersection) so the Brush button stays disabled.
• Fix direction: Keep imageId-intersection gating for STACK viewports only (the
  original blanking / image-index-overlay corruption is stack-specific, and stack
  labelmap actors map per-imageId so cross-series genuinely can't mount —
  syncStackLabelmapActors already gates there). For VOLUME viewports, gate on
  FrameOfReferenceUID match instead of imageId intersection (same-FOR resampling is
  legitimate; cross-FOR stays suppressed, which also matches the OHIF test's
  "FOR2 viewport must NOT show FOR1 seg" expectation). Update the 6g spec's fusion
  cases accordingly.
• Status: DONE. The gate is consolidated into a SINGLE utility that owns the whole
  overlay-compatibility decision (per user request, so future rules land in one place):
  helpers/isSegmentationOverlayCompatible.ts (renamed via git mv from
  viewportReferencesSegmentationImages.ts), signature
  (viewport, segmentationId, representationType).
  • Non-labelmap types (contour/surface, or an OMITTED type — the state manager also
    treats undefined as non-labelmap, so no default param) → always compatible; the
    type === Labelmap check moved from internalAddSegmentationRepresentation into
    the utility.
  • STACK viewports keep the strict imageId-intersection test (stack labelmap actors
    map per-imageId; cross-series genuinely can't mount) — including same-FOR
    different-series stacks, pinned by a new spec case.
  • VOLUME viewports (detected by getAllVolumeIds presence) now gate on
    FrameOfReferenceUID: viewport FOR via getFrameOfReferenceUID() (undefined
    pre-setVolumes / throw → permissive), labelmap FORs from the labelmap volume's
    metadata.FrameOfReferenceUID and/or imagePlaneModule of the referenced
    imageIds (none derivable → permissive). Same-FOR different-series → compatible
    (fixes all 3 OHIF e2e failures); different FOR → suppressed.
  • Both call sites updated (internalAddSegmentationRepresentation,
    syncStackLabelmapActors passes Labelmap explicitly). Spec renamed + rewritten
    (isSegmentationOverlayCompatible.spec.ts, 15 cases incl. representation-type
    passthrough, same-FOR stack suppression, same-FOR volume match, cross-FOR volume
    suppression, volume-backed layer FOR, throw safety for getImageIds and
    getFrameOfReferenceUID). Full tools jest suite passes (34 suites / 634 tests);
    tsc --noEmit clean.
  • Verified against OHIF: layerConfigurationUtils.ts:68-94 coerces the viewport to
    VOLUME whenever a SEG overlay references a different series than the background,
    so the failing data-overlay scenarios hit the FOR rule; a SEG referencing the
    displayed series stays stack and passes the imageId intersection. Re-run the
    downstream "OHIF downstream tests" job to confirm the 3 e2e tests go green.

[-] 4d. No memoization

• Where: viewportReferencesSegmentationImages.ts:52-83, invoked per scroll via
  syncStackLabelmapActors.ts:42
• Problem: Recomputes segmentation lookup + per-image cache reads + fresh Set on
  every stack-scroll/render sync.
• Fix: Cache keyed on (segmentationId, stack signature).
• Status: EVALUATED — mechanics CONFIRMED, severity LOW; recommend won't-fix in
  this PR. The gate does O(layers × labelmapImageIds) cache.getImage Map lookups
  plus a Set build per call, invoked once per stack-scroll sync
  (syncStackLabelmapActors.ts:42) — sub-millisecond even for 1000-slice stacks,
  and dwarfed by the actor scan / vtk image-data work in the same sync. Correct
  memoization is also hazard-prone: the result legitimately changes over time
  (referencedImageIds derived via cache.getImage(...).referencedImageId grows
  as labelmap images land in cache — a cached false could wrongly pin
  suppression) and on setStack, so a safe key must incorporate the referenced-id
  content, which costs most of what it saves. Defer unless profiling shows it.

---

Phase 5 — Minor / polish

[x] 5a. Remove dead RLE code

• Where: decodeRLE.ts — decode1, unpackOneBitInterleaved, interleavedOneBit
  option (sole caller decodeImageFrameWorker.js:386 passes no options).
• Keep: unpackOneBitPlanar — it IS live for the 1-bit path.
• Status: EVALUATED — CONFIRMED dead, safe to remove. decode1 (decodeRLE.ts:70),
  unpackOneBitInterleaved (:36) and the interleavedOneBit option (:9, :20-22)
  are reachable only when the option is passed; the sole decodeRLE caller
  (decodeImageFrameWorker.js:386) passes no options, and no other module in the repo
  imports either function (grep across packages, dist excluded). unpackOneBitPlanar
  is live at :192 (default 1-bit path) — keep. Mechanical deletion, low risk.
  APPLIED: decode1, unpackOneBitInterleaved, and the interleavedOneBit
  option removed from decodeRLE.ts; the 1-bit branch now always takes the
  planar path. unpackOneBitPlanar kept. dicomImageLoader tsc --noEmit clean.

[ ] 5b. mapWithConcurrency NaN → 0 workers → silently empty labelmap

• Where: labelmapImagesFromBuffer.ts:300-301; workers :293-298
• Fix: Coerce/validate limit. Optionally add an AbortController so fail-fast
  cancels in-flight workers.
• Status: EVALUATED — PARTIALLY fixed already; NaN case remains. Current
  mapWithConcurrency (now labelmapImagesFromBuffer.ts:334-353) clamps
  poolSize = Math.max(1, Math.min(limit, items.length)), so 0/negative now yield 1
  worker. But Math.max(1, NaN) is NaN → Array.from({length: NaN}) = [] → zero
  workers → resolves silently with a hole-filled results array. Reachable only when a
  caller passes an explicitly non-finite concurrency (undefined gets the default 16).
  Fix: treat non-finite limit as the default — one line. AbortController: on a frame
  failure Promise.all rejects fast but surviving workers keep draining remaining
  frames in the background — wasted fetches only, no unhandled rejections (Promise.all
  observes every worker). Optional; skip unless doing 6e anyway.

[ ] 5c. prepareSegMultiframeMetadata mutates provider-owned metadata; stale fixed id

• Where: labelmapImagesFromBuffer.ts:44-50; guard at :437 reuses first SEG's
  metadata for cornerstone-adapters-buffer-seg:0
• Fix: Clone before mutating; don't reuse the fixed id across buffer loads.
• Status: EVALUATED — CONFIRMED both halves, low severity.
  (1) Mutation: prepareSegMultiframeMetadata (now :43-51) reshapes
  SharedFunctionalGroupsSequence (array → first element, via
  normalizeSharedFunctionalGroupsSequence) and wraps
  PerFrameFunctionalGroupsSequence in an array — in place, on the object returned by
  metadataProvider.get('instance', …) (instanceMeta.dataset ?? instanceMeta), i.e.
  provider-owned state other consumers may read. The buffer path is unaffected (parses
  its own fresh dataset). Fix: SHALLOW-clone multiframe before normalizing — a deep
  clone would copy PixelData and is wrong.
  (2) Stale fixed id: BUFFER_SEG_IMAGE_ID (:28) + the skip-if-present guard in
  ensureInstanceOnMetadataProvider (:497-499) leaves the FIRST SEG's dataset
  registered under the shared id on every subsequent buffer load. Mitigated in-path —
  the parse uses options.multiframe directly (:1650-1652), so wrong data is not
  read back — it's provider hygiene for external readers. Fix: unique id per load
  (module counter suffix); zero references to the constant outside this file (OHIF
  included, which uses createFromDicomSegImageId instead), and callers can already
  override via options.segImageId.

[x] 5d. Dead allocation + duplicated filter + O(n³) scan

• Where: dead combinedPixelData generateSegmentation.ts:123-127; non-empty-frame
  filter duplicated at :99-111, :258-263, :323-328; labelmapFrameContainsSegment
  O(segments×frames×pixels) at Segmentation_4X.js:81-86
• Fix: Delete dead alloc; extract the filter to one helper; precompute segment
  presence once per frame.
• Status: EVALUATED — MOSTLY CLOSED by 2f; one piece remains. Verified in current
  source: the dead combinedPixelData allocation is gone, the duplicated non-empty
  filter is consolidated into the shared collectNonEmptyFrameIndices/hasAnySegment
  (used by both the export dispatch and fillLabelmapSegmentation), and
  normalizeFramePixelData is removed. REMAINING: labelmapFrameContainsSegment
  (Segmentation_4X.js:89-101) still scans the full frame once per
  (frame, segmentOnLabelmap) pair → O(segments×frames×pixels) worst case. Fix: one
  pass per frame building a Set of present pixel values, then membership checks.
  Small and contained.
  APPLIED: labelmapFrameContainsSegment replaced with
  segmentsPresentInFrame(pixelData) — a single pass per frame collecting the
  Set of non-zero values, checked with presentSegments.has(segmentIndex) in
  the segmentsOnLabelmap loop (still verifies presence, since
  segmentsOnLabelmap can be stale). Also restored the fillSegmentation
  JSDoc's adjacency to its function (the old helper sat between them). Full
  adapters suite passes (17 suites / 94 tests — the 6a/6b round trips exercise
  this loop directly).

[x] 5e. BrushTool guards left after _activateDraw / hideElementCursor

• Where: BrushTool.ts:283-300 (hoverData/operationData guards after draw at ~:264)
• Problem: return false after listeners bound / cursor hidden leaves cursor hidden.
• Fix: Hoist the hoverData/operationData guards above _activateDraw.
• Status: EVALUATED — CONFIRMED (file is tools/segmentation/BrushTool.ts).
  preMouseDownCallback calls _activateDraw(element) (:264) and
  hideElementCursor(element) (:266) before the _hoverData guard (:284-286) and
  the operationData guard (:298-300); return false on either leaves the drag
  listeners bound and the cursor hidden. Fix: compute
  createHoverData/getOperationData first and early-out before
  _activateDraw/hideElementCursor (neither depends on _activateDraw's side
  effects — it only binds listeners), or symmetrically deactivate/restore on the
  early-out paths.
  APPLIED (as sink, not hoist): getOperationData reads brushCursor state
  that _calculateCursor populates, so hoisting the guards above the whole
  block would have changed the points passed to OnInteractionStart.
  Instead the three side-effect calls — _activateDraw, hideElementCursor,
  evt.preventDefault() — moved BELOW the last guard (after operationData),
  preserving every other statement's relative order. Everything now running
  before the guards is state/render-only, so a return false leaves no bound
  listeners and no hidden cursor. Tools tsc --noEmit clean; all tools jest
  suites pass (26 suites / 203 tests).

---

Phase 6 — Tests (land alongside their corresponding fixes)

Reminder: the dev-script sourcemap/inlineSources changes, undici override, and
scroll-info fix are the test infrastructure that makes these runnable — keep them.

[x] 6a. Real dcmjs SegmentationDerivation round-trip test

• Catches 1d + 1e (both critical write-path bugs). Current round-trip test stubs
  createAndCacheDerivedLabelmapImage.
• Status: EVALUATED — STILL OPEN; highest-value remaining test. Verified:
  segBufferMetadataRoundTrip.jest.js is read-path only (it hand-builds the SEG
  Part 10 with dcmjs DicomDict, never through generateSegmentation; the only stub
  is the derived-labelmap image factory), and labelmapSegmentationExport.jest.js
  tests exported helpers only. Nothing anywhere runs generateSegmentation → real
  SegmentationDerivation/derive() → read-back, so 1d (plane sequences after
  derive) and 1e (lesion starting at slice 10) are pinned only by inspection.
  Feasible in jest: all dcmjs parts run in node; needs image stubs with
  voxelManager.getScalarData() and a metadata stub serving
  STUDY_DATA/SERIES_DATA/IMAGE_DATA/IMAGE_PLANE modules. Moderate effort. Assert:
  per-frame PlanePosition/PlaneOrientation present + correct per slice (1d), export
  succeeds and geometry aligns when the first non-empty frame is > 0 (1e), and
  BitsAllocated===16 for labels > 255 (closes 2f's deferred assertion).
  DONE. test/segRealDerivationRoundTrip.jest.js + shared fixtures in
  test/helpers/segRoundTrip.js (12-slice synthetic CT stack, full metadata
  provider, Part 10 writer). Two round trips, both through real dcmjs
  normalize/derive → fill → encode → Part 10 write → read →
  createFromDICOMSegBuffer → labelmap insert: (1) BINARY lesion on slices
  10–11 only — no throw (1e), per-frame source refs AND plane positions match
  slices 10/11, pixels read back onto the right slices; (2) LABELMAP with
  frames on slices 3 and 10 (one multi-label) — real plane sequences after
  derive() (1d), no SegmentIdentificationSequence (2h), multi-label pixels
  round-trip. NEW FINDING (follow-up, not fixed): dcmjs'
  ImageNormalizer sorts the virtual multiframe's frames by DESCENDING distance
  along the scan axis, and the BINARY path copies each frame's
  PlanePositionSequence from the normalized multiframe indexed by ORIGINAL
  image order — so for an ASCENDING-ordered input stack the exported BINARY
  plane positions belong to mirrored slices (source-ref UIDs, which
  cornerstone's reader uses, stay correct; position-based readers like Slicer
  would misplace). The test pins the self-consistent descending case and
  documents the assumption in the fixture helper. LABELMAP is immune (1d
  rebuilds planes from each image's own imagePlaneModule — pinned with an
  ascending stack on purpose). Predates-vs-PR status unverified; triage
  separately.

[x] 6b. 16-bit and LABELMAP SEG round-trip test

• Catches 1b. Nothing today uses parserType:'labelmap' or exercises
  processLabelmapChunk; 16-bit exists only as encode-side unit tests.
• Status: EVALUATED — STILL OPEN. Confirmed by grep: zero test references to
  parserType/processLabelmapChunk/mapWithConcurrency in packages/adapters/test;
  16-bit is covered only at the encode/decode unit level (encodePixelData.jest.js,
  from 1b). Natural shape: a LABELMAP + 16-bit variant of the 6a round-trip (or
  standalone buffer → createLabelmapsFromDICOMBuffer with parserType: 'labelmap'),
  asserting segment values survive, including one > 255.
  DONE — and it caught two REAL working-tree bugs (both fixed):
  test/segLabelmap16BitRoundTrip.jest.js — 16-bit LABELMAP (labels 3 and 300)
  through Explicit VR LE, RLE Lossless (exercises 1b's
  decodeSegFramesFromMultiframe 16-bit RLE branch end-to-end), and an explicit
  parserType: 'labelmap' run through processLabelmapChunk; asserts
  BitsAllocated/BitsStored/HighBit 16/16/15 (2f's deferred assertion) and that
  300 survives (not 300 % 256 = 44).
  • Bug 1 (completes 1b): getBitmapFramesFromDataset's 16-bit branch did
    new Uint16Array(dataset.PixelData as ArrayBuffer), but a naturalized
    Part 10 dataset carries PixelData as [ArrayBuffer] — the array-like
    constructor built a 1-element garbage array (crash: "Offset and length out
    of bounds"). Now unwraps via asUint8PixelData (like the 8-bit branch) and
    reinterprets the raw LE bytes. The prior 1b unit tests fed bare
    buffers/typed arrays, so they never saw the naturalized shape.
  • Bug 2 (completes 1c): readFromUnpackedChunks built Uint8Array views
    using element-based offsets — correct for 8-bit chunks, but 16-bit
    (Uint16Array) chunks were read as BYTES (300 → [44, 1] on adjacent
    pixels). Now constructs views of the chunks' own element type with offsets
    scaled by BYTES_PER_ELEMENT; 8-bit/packed callers are unchanged
    (readFromUnpackedChunks.jest.js still green).
    Full adapters suite passes (17 suites / 94 tests).

[x] 6c. Per-frame image-loader-path test

• createLabelmapsFromSegImageIds through real loadImage + mapWithConcurrency
  (bounded-concurrency multi-frame fetch is untested).
• Status: EVALUATED — STILL OPEN, and value is HIGHER than the review scored it:
  this is OHIF's production path (getSopClassHandlerModule.ts:515 calls
  createFromDicomSegImageId → createLabelmapsFromSegImageIds →
  defaultDecodeFrameImageData → imageLoader.loadImage), yet every existing test
  injects a decodeImageData stub, so defaultDecodeFrameImageData and the
  loader hop have zero coverage. Feasible: register a fake image-loader scheme with
  imageLoader.registerImageLoader in jest and drive a small multi-frame SEG through
  it, asserting frame order and that at most concurrency loads are in flight.
  DONE. test/segImageLoaderPath.jest.js: registers a segframes: loader
  with the REAL imageLoader.registerImageLoader, serves the SEG instance from
  the metadata provider (production shape), and drives
  createFromDicomSegImageId with concurrency: 2 over a 3-frame LABELMAP SEG
  whose later frames resolve FASTER than earlier ones. Asserts: every frame
  fetched through the real loader (defaultDecodeFrameImageData →
  imageLoader.loadImage), loads overlap but max-in-flight === 2, and frames
  stay index-aligned despite out-of-order completion (also covers 6e's
  order-preservation concern at the integration level).

[x] 6d. Multiframe-source ReferencedFrameNumber load-side test

• getImageIdOfSourceImageBySourceImageSequence (Segmentation_4X.js:1525-1566) is
  untested. Pairs with 2i.
• Status: DONE — closed by 2i's test. sourceImageFrameMapping.jest.js imports
  getImageIdOfSourceImageBySourceImageSequence from Segmentation_4X directly and
  asserts every frame of a multiframe wadouri AND dicomfile source resolves to its own
  source frame (wadors as passing control). No further work needed.

[ ] 6e. mapWithConcurrency behavior test

• Order preservation, deadlock-free, NaN/edge limits. Pairs with 5b.
• Status: EVALUATED — STILL OPEN. Note mapWithConcurrency is NOT exported from
  labelmapImagesFromBuffer.ts; either export it for testing (matching the file's
  existing pattern of test-only exports) or test through the exported
  decodeSegPixelDataFromFrameIds with a stub decodeImageData (order preservation,
  bounded in-flight count, single-frame path, 0/negative/NaN limits). Land together
  with the 5b NaN fix so the edge-limit cases assert the fixed behavior.

[x] 6f. packBits control-byte boundary tests

• Runs of exactly 128/129, -128 no-op, odd-length padding, row boundaries.
  Pairs with 2g.
• Status: DONE — closed by 2g (test/packBits.jest.js: 128/129 runs, 128-literal,
  no -128 no-op, per-row split vs whole-segment, non-dividing rowLength fallback,
  row-by-row round-trip, even-length padding).

[x] 6g. viewportReferencesSegmentationImages spec

• No test exists today despite it being a behavioral gate. Cover empty/fusion volume
  viewports. Pairs with Phase 4.
• Status: DONE. Colocated jest spec
  src/stateManagement/segmentation/helpers/viewportReferencesSegmentationImages.spec.ts
  (12 cases): permissive fallbacks (no viewport / unknown segmentation / no
  referenced ids derivable), stack match/suppress + referenced-ids-derived-from-
  cached-labelmap-images path, volume viewports (empty = pre-setVolumes →
  permissive without touching getImageIds; evicted volume → permissive; CT+PT
  fusion matching a PT-derived labelmap; fusion suppressing an unrelated one),
  and throw-safety for both getImageIds and getAllVolumeIds. The volume
  mocks' getImageIds throws by construction (as the real
  BaseVolumeViewport does pre-actor), so these cases fail against the pre-fix
  gate. All segmentation stateManagement suites pass (7 suites / 66 tests);
  tsc --noEmit clean (spec files are build-excluded by tsconfig).
  SUPERSEDED by 4e: spec renamed to isSegmentationOverlayCompatible.spec.ts and
  volume cases rewritten to frame-of-reference semantics (see 4e status).

[ ] 6h. Fix tests that pin buggy behavior

• resolveFrameImageIds.jest.js:76-88 asserts the repeated-base-id fallback "as
  documented" — retarget once 2k throws.
• perFrameFunctionalGroups.jest.js:108-120 asserts the frame-drop/NumberOfFrames-shrink
  path (dead in real export) — reframe.
• Status: EVALUATED — first bullet DONE (2k retargeted
  resolveFrameImageIds.jest.js to assert the throw + single-frame passthrough).
  Second bullet: the "drops frames without a referenced SOP instance UID" case (still
  at :108-120) pins helper-level behavior that is now unreachable from BOTH export
  drivers — fillSegmentation throws first, and fillLabelmapSegmentation's
  perFrameInputs builder throws for an unresolvable UID
  (generateSegmentation.ts:317-323). The drop branch remains a safety net for
  direct applyPerFrameFunctionalGroups callers, so the test isn't wrong — reframe =
  add a comment stating the branch is dead in the export paths and exists for direct
  callers only. Making the helper throw instead is a behavior change out of scope.

---

Release / compatibility notes (verified 2026-07-09)

Two-step rollout is viable: (A) release this cs3d PR and validate it under
OHIF origin/master, then (B) land the OHIF-side changes against that
released cs3d.

A. This cs3d branch + OHIF origin/master — compatible

• SEG load: master calls
  adaptersSEG.Cornerstone3D.Segmentation.createFromDICOMSegBuffer(imageIds, arrayBuffer, { metadataProvider, tolerance }) and consumes
  segMetadata.data / labelMapImages (flattened in
  SegmentationService.ts:469-492). The branch keeps that exact contract (the
  wrapper still takes a Part 10 ArrayBuffer), and master — pinned to cs3d
  5.1.3 — already consumes the new return shape; labelmapBufferArray appears
  nowhere on master. The 2c break only affects SEG generateToolState, which
  master never calls (its only generateToolState uses are the PMAP and SR
  adapters, untouched by this PR). 2e's renamed internal has no consumers.
• SEG export: master calls generateSegmentation(referencedImages, labelmap3D, metaData, { predecessorImageId }) with no format options →
  with 2a's restored defaults this produces BINARY (66.4) + Explicit VR LE,
  identical to pre-PR output. The Label Map + RLE default lives in OHIF's
  getSegmentationSaveOptions (branch-only), so master is unaffected.
• Loader: the WADO-RS registry lookup no-ops when nothing is prefetched
  (master never calls prefetchPart10Instance); the 3c network fallback only
  adds robustness; 2k's throw is on the imageId path master doesn't use.
• Tools: the Phase 4 gate (as fixed) is permissive in every ambiguous
  case; master's flows change only by the intended suppression of
  genuinely-unrelated viewports.
• ⚠️ Memory caveat (wadouri-style datasources only): this branch
  re-registers the wadouri: / dicomweb: / dicomfile: schemes to
  loadImageFromNaturalizedMetadata (wadouri/register.ts:34-37), which
  parses full instances into the NATURALIZED registry, and nothing on that
  path evicts them. Master OHIF never calls clearCacheData() (that call is
  the 3a fix, OHIF-branch-only), so a master-OHIF instance accumulates full
  parsed instances for the browser session on WADO-URI/local-file
  datasources. WADO-RS deployments (the usual case) only see the trivial
  per-frame BASE_IMAGE_ID string growth (3b) and are fine.
• Pinning: master pins exact cs3d versions (5.1.3), so this test requires
  linking the branch build (or a resolutions override) — meaning any
  unrelated 5.2→5.4 changes ride along in the test, not just this PR's diff.

B. OHIF-side changes require a cs3d release that includes this PR

The OHIF branch hard-depends on three APIs that do not exist in cs3d without
this PR: dicomImageLoader.prefetchPart10Instance
(DicomWebDataSource/index.ts:379 — unguarded, throws against older cs3d),
createFromDicomSegImageId (getSopClassHandlerModule.ts:515), and
@cornerstonejs/metadata utilities.clearCacheData (the new onModeExit
release). So step B must pin to the release produced in step A.

---

Suggested order & dependencies

1. Phase 1 first (silent corruption). 1b ties to 6b; 1d+1e share the derivation code.
2. Phase 2 — decide the back-compat policy (restore vs document) before coding.
   2g↔6f, 2i↔6d, 2k↔6h.
3. Phases 3 & 4 are independent of 1/2 and each other — parallelizable.
4. Phase 5 polish; land Phase 6 tests with their matching fixes.

[wayfarer3130]

@sedghi # PR #5806 — Review Remediation Plan (OHIF side)

PR: feat: Add support for labelmap seg images in any supported tsuid (also for compressed bitmap)
Branch: feat/load-seg-images → master — OHIF/Viewers#5806
Companion: #2611 (fix/use-imageLoader-for-seg, see
libs/@cornerstonejs/PR-2611-review-plan.md)

Every item below was verified against the current checkout (top commit 22c7c60c24,
"Bump @cornerstonejs/* pins 5.1.3 -> 5.4.10") with 5 parallel verification passes on
2026-07-09. Line numbers are the actual current locations. The review was written
against an older head (pre-bump), so one claim is partially stale — noted inline.

Work through items one at a time. Check the box and fill in Status: as you go.

Legend: [ ] todo · [~] in progress · [x] done · [-] won't-fix / documented instead

---

Notes on scope (corrections to the review, confirmed during verification)

• Bug 3 (version pins) is NOT a plan item — it's the designed CS3D_REF workflow.
  Referencing an unreleased cs3d is the entire point of the repo's CS3D integration
  mechanism: CI tests against a cs3d branch (cloned into libs/@cornerstonejs, built,
  linked) when the PR carries the ohif-integration label + a CS3D_REF: line, and the
  CS3D Branch Merge Guard hard-fails merge while the ref is a branch — so the
  "registry lacks the APIs" state is expected and CI-enforced, not a bug. The review's
  factual halves were still checked: the "no bump in the diff" half is stale (commit
  22c7c60c24 bumps 10 package.json files + lockfile to 5.4.10 consistently), and
  published 5.4.10 indeed lacks createFromDicomSegImageId / prefetchPart10Instance
  (tarballs unpacked and grepped) — which is exactly the state the merge guard exists
  for. What remains is CI wiring on the PR itself (see 1c).
• Bug 1's blast radius is narrower than "every SEG/SR saved": the remote STOW upload
  path (DicomWebDataSource/index.ts:474) builds its own hex-keyed dict via
  denaturalizeDataset + writeDicomDictToPart10Buffer and never runs
  applyTransferSyntaxToFileMeta — server copies are clean. Affected: download,
  copy-to-clipboard, the local-wadouri blob, and DicomLocalDataSource.store.dicom.
• Bug 4 reaches the wire (STOW bytes, download, clipboard) but NOT the local wadouri
  cached copy — the local blob is built before the mutation, so stored copy ≠ local
  copy (an internal inconsistency on top of the conformance issue).
• Blob-leak risk: the review said "blob URLs are never released" — slightly imprecise;
  no URL.createObjectURL is involved (the dicomfile:N scheme indexes an array), it's
  Blob references that accumulate. Same memory effect. Also: fileManager.remove() /
  purge() already exist in the loader — OHIF just never calls them.
• settle-before-apply risk: the promise does already wait for SEG pixel-data load
  (SEGMENTATION_LOADING_COMPLETE fires after addOrUpdateSegmentation); early-resolve
  only skips the representation attachment phase. Trivial case = a microtask; the real
  window is the async stack→volume viewport conversion inside applyRepresentation.
• dcmjs engines risk is effectively refuted for this repo: the repo's own
  engines.node >= 24 (root package.json, .node-version 24.15.0, all CI at 24.15.0)
  is stricter than dcmjs's >= 22.13, and pnpm only warns on dependency engines
  (no engine-strict anywhere). No install can fail because of dcmjs here.
• frameNumber non-enumerable risk: mechanism confirmed, but a repo-wide consumer
  audit found zero currently-broken readers — one risky copy site exists
  (SegmentationService.ts:511-514) whose copies nothing reads .frameNumber from
  today. Latent footgun, not an active bug. A related, un-reviewed hazard was found
  in the same commit (see 3a).

---

Phase 1 — Confirmed bugs (block merge)

[x] 1a. Garbage (0000,0000) element written into every saved file's meta group

• Where: extensions/default/src/utils/dicomWriter.ts:90
  (dicomDict.meta.TransferSyntaxUID = entry; — the hex assignment at :91 is correct)
• Verified: CONFIRMED with a runtime repro against installed dcmjs 0.52.0.
  DicomMessage.write iterates ALL meta keys; Tag.fromString('TransferSyntaxUID')
  parses to (0000,0000) via parseInt(..., 16) → NaN → 0. The re-read file meta
  contains an extra '00000000' UI element, it is counted in
  FileMetaInformationGroupLength, and (since 'T' sorts after hex digits) it is
  written after the 0002 elements — doubly non-conformant (garbage tag + out-of-order
  element in group 2). dcmjs tolerates re-reading it; dciodvfy/strict archives may not.
• Affected flows (all via datasetToDicomBlob → datasetToDicomPart10Buffer):
  • commandsModule.ts:793 — Download DICOM
  • commandsModule.ts:806 — Copy to clipboard
  • registerNaturalizedDatasetForLocalWadouri.js:26 — local wadouri blob (invoked from
    all three createStoreFunction branches, incl. the STOW path at :824 — the local
    blob dicomParser must then re-parse carries the garbage element)
  • DicomLocalDataSource/index.js:183 — local data source store
  • NOT affected: remote STOW upload (DicomWebDataSource/index.ts:474, separate
    writer path).
• Fix: Delete line 90 exactly as the review says. Keep line 91 — it is NOT redundant
  with datasetToDict: dcmjs only honors the object-form _meta.TransferSyntaxUID,
  while OHIF's getDatasetTransferSyntaxUID also handles plain-string _meta and
  top-level dataset.TransferSyntaxUID fallbacks that dcmjs would silently default to
  Explicit VR LE.
• Pairs with: 4a (regression test — the existing dicomWriter.test.ts never calls
  datasetToDicomPart10Buffer/applyTransferSyntaxToFileMeta; it only tests
  writeDicomDictToPart10Buffer fragmentation).
• Status: FIXED 2026-07-09 — deleted the naturalized-key assignment, kept the hex
  assignment with a comment explaining why hex-only. Regression test added (4a);
  mutation-checked (re-introducing the line fails both new meta tests).

[x] 1b. rsbuild config crashes for any fresh clone (source-map-loader not a dependency)

• Where: rsbuild.config.ts:82-87 — unconditional
  require.resolve('source-map-loader') at :85, evaluated at config-load time; no
  try/catch, no existsSync guard.
• Verified: CONFIRMED, and worse than reviewed: source-map-loader appears in
  zero package.json files in the working tree (not even an uncommitted local change —
  git status is clean), is absent from the committed pnpm-lock.yaml, and exists only
  as a stray manually-placed directory in local node_modules (v4.0.2, not a pnpm
  symlink). The include regex targets libs/@cornerstonejs/packages/*/dist/esm, which
  is gitignored local cs3d-linking tooling (.gitignore:65,71; git ls-files libs
  → 0 files) — harmless by itself (never matches), the crash is purely the
  require.resolve. Added in commit f81c8b4333 ("Debug fixes"), in the PR range.
• Fix: Drop the hunk from the PR (it's personal debug tooling). If it should be
  shared: wrap the resolve in try/catch (or gate on
  fs.existsSync(path.join(__dirname, 'libs/@cornerstonejs'))) AND add
  source-map-loader to root devDependencies + lockfile.
• Status: FIXED 2026-07-09 — chose the guard (not the drop): require.resolve
  wrapped in try/catch at module top (SOURCE_MAP_LOADER), rule conditionally spread
  only when resolvable. Deliberately did NOT add it to devDependencies — that needs a
  pnpm install, which clobbers cs3d:link symlinks; decide separately if it should be
  a real dep. Config parse-checked (rsbuild loads configs transpile-only).

[X] 1c. Wire up the CS3D_REF integration on PR #5806 (review's "unreleased APIs" item, reframed)

• Not a code bug. Depending on unreleased cs3d is by design — the CS3D_REF workflow
  exists precisely for this (README :182-217,
  platform/docs/docs/development/cs3d-integration.md): CI clones the cs3d branch into
  libs/@cornerstonejs, builds and links it, runs Playwright against it, and the
  CS3D Branch Merge Guard (.scripts/ci/cs3d-branch-merge-guard.sh) fails while the
  ref is a branch, preventing merge until re-run against a published version.
• The actual gap (verified against the live PR): the integration isn't currently
  triggered for #5806:
  1. The ohif-integration label is not on the PR (label list is empty) — without
     it cs3d-check-integration.sh disables the integration path entirely.
  2. The PR body mentions the ref as inline lowercase text
     (cs3d_ref: `fix/use-imageLoader-for-seg` inside a sentence), but the parser
     (cs3d-check-integration.sh:17-18) only matches an uppercase CS3D_REF: at the
     start of a line — so even with the label, CI would fall back to the default
     (4.19+), which lacks the new APIs and would fail misleadingly.
• Note on .CS3D_REF file: verified there is no file-based mechanism in either
  repo (grepped OHIF + cs3d trees, CI scripts, workflows, docs) — the ref is carried
  only by the PR body line (CS3D_REF: here; symmetric OHIF_REF: on the cs3d side).
  A .CS3D_REF file would be read by nothing. If a checked-in file is preferred over
  the PR body (it does survive body edits and is visible in the diff), that's a small
  CI change: teach cs3d-check-integration.sh to prefer a repo-root .CS3D_REF file
  over the body line — optional, separate from this PR.
• Fix:
  1. Add the ohif-integration label to #5806.
  2. Add a properly formatted line to the PR body (own line, uppercase):
     CS3D_REF: fix/use-imageLoader-for-seg
     (if the cs3d branch lives on a fork, use the owner:branch form).
  3. At merge time (once cs3d fix: Use the image loader for dicom-seg to allow compressed images #2611 is released): update the body line to the release
     version, re-run so the merge guard goes green, and apply the pins via
     node .scripts/cs3d-set-version.mjs <version> + commit — matching step B of the
     two-step rollout in the PR-2611 plan.
• Supporting facts kept from verification: published 5.4.10 tarballs lack both
  APIs; createFromDicomSegImageId call site is unguarded
  (getSopClassHandlerModule.ts:515, try/finally with no catch) while
  prefetchPart10Instance degrades gracefully (DicomWebDataSource/index.ts:377-388).
  No code change needed — the merge guard is the mechanism that prevents a registry
  install of this state from ever shipping.
• Status:

[x] 1d. Bogus NumberOfFrames stamped onto SR/RTSTRUCT before store/download/clipboard

• Where: extensions/default/src/utils/registerNaturalizedDatasetForLocalWadouri.js:44
  — dataset.NumberOfFrames = numberOfFrames (min 1), plain enumerable assignment,
  unconditional. The makeExistingPropertiesNonEnumerable call at :66 does NOT cover
  it (NumberOfFrames is not in RUNTIME_INSTANCE_PROPERTY_KEYS, dicomWriter.ts:18-29).
• Verified: CONFIRMED. All 3 callers are in createStoreFunction
  (commandsModule.ts:788 download, :801 clipboard, :824 STOW), and each runs the
  mutation before serialization — store.dicom denaturalizes the same mutated
  object (DicomWebDataSource/index.ts:469) and STOWs it (:481); dicomDict is
  never supplied on the SEG/RTSS path, so no bypass. Affected IODs: SR (measurement
  reports) and RTSTRUCT gain a bogus (0028,0008) = 1 in the wire/download/clipboard
  bytes; SEG is harmless (legitimately multiframe). Nuance: the local wadouri blob is
  built at :26, before the mutation, so local copy ≠ stored copy.
• Fix: Keep the computed count in a local variable (the per-frame loop at :57-63 is
  the only consumer) and only write dataset.NumberOfFrames when the dataset is actually
  multiframe (Array.isArray(PerFrameFunctionalGroupsSequence) — the :35-37 branch
  already does this) or has PixelData. If the provider truly needs it on the dataset,
  use setNonEnumerableInstanceProperty instead. Local-variable option is cleanest.
• Pairs with: 4b (test asserting SR datasets don't gain the tag).
• Status: FIXED 2026-07-09 — took the local-variable option: removed the
  unconditional dataset.NumberOfFrames = write; the :35-37 multiframe branch
  remains the only dataset stamp. Regression test added (4b); mutation-checked.

---

Phase 2 — Design concerns (confirmed; decide + fix)

[x] 2a. Every report save permanently retains a full Part 10 Blob (and more)

• Where: registerNaturalizedDatasetForLocalWadouri.js:26-27 →
  dicomImageLoader.wadouri.fileManager.add(blob); all three createStoreFunction
  branches call it (SR, SEG, RTSS; download/clipboard/STOW alike).
• Verified: CONFIRMED. What accumulates per save, forever:
  1. The full Part 10 Blob in the fileManager's module-level files: Blob[] (SEG: many
     MB; SR/RTSS: KB–MB). remove(index) / purge() exist
     (libs/@cornerstonejs/packages/dicomImageLoader/src/imageLoader/wadouri/fileManager.ts:13-19)
     but OHIF only ever calls add (3 sites; zero remove/purge calls anywhere,
     including onModeExit).
  2. If the dicomfile: id is later loaded: a second full byte copy in
     dataSetCacheManager.loadedDataSets (unload/purge exist, also never called).
  3. Per-frame imageURI → UIDs entries in MetadataProvider.imageURIToUIDs (Map, no
     removal API; one entry per frame per save for multiframe SEG).
     (Review's "blob URLs" wording imprecise — no URL.createObjectURL; Blob references
     via the dicomfile:N index. Same memory effect.)
• Fix: Two complementary moves:
  1. Narrow registration to datasets that need local pixel loading (SEG / has
     PixelData) — SR/RTSS never need a wadouri image-loader entry (review's suggestion,
     confirmed correct).
  2. Wire up release: record the returned imageId index and call
     fileManager.remove(index) + dataSetCacheManager.unload(uri) on display-set/
     study close or onModeExit (the APIs exist; symmetric with the cs3d-side 3a fix —
     clearCacheData() in onModeExit — already applied for the metadata registry).
• Status: FIXED 2026-07-09 — both moves: (1) registration skips datasets with no
  PixelData and no PerFrameFunctionalGroupsSequence (SR/RTSS — verified no reader of
  .url on those IODs); (2) registered imageIds are tracked and the new
  releaseLocalWadouriRegistrations() (fileManager.remove + dataSetCacheManager
  unload-to-zero) runs from extensions/default onModeExit. Tests updated (skip /
  register / release / idempotence). imageURIToUIDs growth left as-is (no removal
  API; now SEG-only, bounded per session).

[x] 2b. Viewport-ready promise resolves before the SEG representation is applied

• Where: extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
addOverlayRepresentationForDisplaySet (:1285-1388): in the
  SEGMENTATION_LOADING_COMPLETE handler, settle() at :1345 runs before
  await applyRepresentation() at :1348; apply failures downgraded to console.warn
  (:1350-1353). Timeout (SEG_LOADING_TIMEOUT_MS = 120000, :1325) and
  loading-failure paths intentionally settle without applying.
• Verified: CONFIRMED, with an important nuance: the promise already waits for the
  SEG pixel-data load; early-resolve skips only representation attachment. Trivial
  case = one microtask. The real window: applyRepresentation can trigger an async
  stack→volume conversion (handleStackViewportCase → convertStackToVolumeViewport
  → wait for GRID_STATE_CHANGED), during which the awaiters proceed:
  _setStackViewport:943 / _setVolumeViewport:1219 continue into
  rotation/flip/render()/setPresentations, and setViewportData's .then broadcasts
  VIEWPORT_DATA_CHANGED (:543-548) — consumed by the hanging-protocol
  onViewportDataInitialized counter (commandsModule.ts:1469-1484), toolbar refresh,
  overlays, e2e readiness — all before the labelmap actor exists. A readiness-signal lie:
  screenshot/e2e flake + protocol callbacks acting on an overlay-less viewport; plus
  swallowed apply failures are invisible to callers. Not data corruption.
• Fix (with caveats — do not naively move settle): keep settled = true +
  unsubscribe where they are (dedupe), move only resolve() into a finally after the
  await, keep resolve-not-reject (best-effort semantics), and do not clear the 120 s
  timeout until final resolve so a hung applyRepresentation stays bounded. Known
  re-entrancy hazard to test: convertStackToVolumeViewport →
  setDisplaySetsForViewport → a new setViewportData for the same viewport while
  the old _setStackViewport is suspended at :943 — resolving after apply means the
  stale stack-setup continuation runs against a rebuilt volume viewport and the old
  VIEWPORT_DATA_CHANGED fires after the replacement setup began. No hard deadlock
  (GRID_STATE_CHANGED is emitted independently), but verify event ordering in the
  conversion scenario before landing. Timeout/failure paths need no change.
• Status: FIXED 2026-07-09 exactly per the caveats — settled + unsubscribe stay
  at handler entry (dedupe), resolve() moved to a finally after
  await applyRepresentation(), timeout NOT cleared until final resolve (a hung
  apply resolves at the 120 s bound with a distinct warn; double-resolve is a no-op).
  Load-failure / timeout-before-event paths unchanged (settleWithoutApply).
  Stack→volume conversion re-entrancy still needs a runtime check (4c open).

[x] 2c. loadMultiframeAsPart10RaceTimeMs: configs ship 3000 ms, doc recommends 250–750 ms

• Where: platform/app/public/config/default.js:91 and config/e2e.js:118,159,187,214,244,276,309
  (all 3000); doc platform/docs/docs/behaviours/segmentation-multiframe-part10-prefetch.md:312-313
  recommends 250–750 ms.
• Verified: CONFIRMED, and the cost is real, not cosmetic:
  getSopClassHandlerModule.ts:509 does await Promise.race([prefetch.done, raceTimer])
  before createFromDicomSegImageId(:515) — frame decode does NOT start in parallel,
  and registration is not progressive (whole-instance fetch+parse before any frame is
  registered; progressive registration is doc'd as future "rollout step 4"). So on any
  origin where the full-instance fetch takes > 3 s, time-to-first-SEG-render is delayed
  by a flat 3 s with zero offsetting benefit at decode start. Fast origins (e2e
  static-wado) are unaffected — the fetch wins before the timer matters.
• Fix: Either lower the shipped configs into the doc's 250–750 range, or keep 3000
  and add a justifying comment in both configs (e.g. demo servers: bulk fetch usually
  wins and avoids hundreds of per-frame requests) — and reconcile the doc's
  recommendation to match whichever is chosen. Pure latency/bandwidth tuning; no
  correctness risk either way.
• Status: Lets just have the back end datasource configuration indicate that this is the way to fetch the data, and if so, wait until ti is done, OR it fails, and don't bother with a timeout at all here.
  IMPLEMENTED 2026-07-09 per the directive above — the knob is now the boolean
  loadMultiframeAsPart10 (data source config, then
  cornerstone.segmentation.loadMultiframeAsPart10 customization, default true):
  when on, the SEG load awaits prefetch.done (completes or fails — no timer at
  all); false is the explicit per-frame exception. Race-timer code, the
  loadMultiframeAsPart10RaceTimeMs param, and all config occurrences removed;
  behaviours doc + README rewritten to match. Also corrected the bogus "static WADO
  has no efficient per-frame endpoint" comments — the endpoint is efficient, the win
  is that SEG frames are tiny and numerous, so one bulk fetch beats hundreds of tiny
  requests (probed the dev study: the SEG instance is ~424 MB, which is also why any
  finite race cap would always lose and storm per-frame).

---

Phase 3 — Latent / informational (document or micro-fix; don't block)

[-] 3a. frameNumber made non-enumerable — latent footgun, zero active breakage

• Where: platform/core/src/utils/combineFrameInstance.ts:105-110 (multiframe) and
  :150-155 (RTDOSE) — enumerable: true → false on this branch (commit "feat: load
  DICOM SEG images via imageLoader"); motive is dcmjs serialization cleanliness
  (dicomWriter.ts:17-29 RUNTIME_INSTANCE_PROPERTY_KEYS + makeExistingPropertiesNonEnumerable).
• Verified: repo-wide audit of every .frameNumber consumer (measurement mappings,
  getSOPInstanceAttributes, MetadataProvider, SR hydration, SEG handlers, data sources,
  sortStudy, plus cs3d's MeasurementReport.ts:651 direct-read path): all read
  directly off originals or plain objects — none break. Exactly ONE risky copy site:
  extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts:511-514
  spreads metaData.get('instance', image.referencedImageId) — for frame-qualified
  multiframe ids (exactly what this branch enables) the copy silently drops
  frameNumber; but no consumer of displaySet.images reads .frameNumber today.
• Related hazard found (not in the review): the branch also makes
  instance.imageId/url/wadoRoot/wadoUri non-enumerable
  (setNonEnumerableInstanceProperty, e.g. DicomWebDataSource/index.ts:634-635,661),
  and combineFrameInstance.ts:85,118 builds _parentInstance via { ...instance } —
  so combined frame instances now lose those props from the parent copy. No current
  reader found either; same class of latent break.
• Fix (cheap insurance): in SegmentationService.ts:511-514, don't spread the
  instance — keep a reference, or re-attach explicitly
  (Object.assign({}, image, inst, { frameNumber: inst?.frameNumber })). Optionally add
  a copyInstance() helper that re-applies RUNTIME_INSTANCE_PROPERTY_KEYS after any
  copy, and a one-line comment on combineFrameInstance that instances from
  metaData.get('instance', …) must not be spread. Acceptable to defer with a note in
  the PR description.
• Status: Document, don't change this. The change was intentional since the frameNumber should NOT be copied.
  DOCUMENTED 2026-07-09 per the directive above — no behavior change. Added a note at
  the SegmentationService spread site stating the spread deliberately does NOT copy
  the non-enumerable runtime props (frameNumber in particular must not be carried
  onto derived objects; read it off the original instance), and a matching note in
  the combineFrameInstance JSDoc.

[-] 3b. dcmjs 0.52.0 engines node >= 22.13

• Verified: premise true (installed dcmjs 0.52.0 declares it; the 0.49.4 → 0.52.0
  bump + lockfile override are on this branch), but the risk cannot bite this repo:
  root engines.node >= 24 is stricter and is what pnpm actually hard-enforces;
  .node-version and every CI/Netlify pin are 24.15.0; no engine-strict anywhere, so
  dependency engines only warn. (Note: repo is pnpm 11 — the CLAUDE.md "yarn" text is
  stale for this fork.)
• Action: none. Optionally note in the PR description that dcmjs 0.52.0 raises the
  floor to Node 22.13 for downstream consumers of the published OHIF packages.
• Status: WON'T-FIX (documented here).

---

Phase 4 — Tests (land alongside their fixes)

[x] 4a. Regression test for datasetToDicomPart10Buffer file meta (pairs with 1a)

• dicomWriter.test.ts currently only exercises writeDicomDictToPart10Buffer
  fragmentation with a hand-built hex-keyed dict — applyTransferSyntaxToFileMeta /
  datasetToDicomPart10Buffer / datasetToDicomBlob have zero coverage, which is why
  1a shipped. Add: build a naturalized dataset (string-form _meta.TransferSyntaxUID
  too, to pin the fallback that makes line 91 necessary), run
  datasetToDicomPart10Buffer, re-read with DicomMessage.readFile, assert meta keys
  are exactly the 0002 group (no '00000000'), '00020010' matches, and
  FileMetaInformationGroupLength is consistent. (A working repro script exists from
  verification — trivially convertible to a jest test.)
• Status: DONE 2026-07-09 — two tests in dicomWriter.test.ts: (1) byte-level meta
  walk (all tags group 0002, ascending, group length spans exactly) + dcmjs re-read
  (no '00000000', '00020010' matches); (2) string-form _meta.TransferSyntaxUID
  with Implicit VR LE pins the fallback that makes the hex assignment necessary.

[x] 4b. SR datasets not mutated by local-wadouri registration (pairs with 1d)

• Feed a minimal SR (no PixelData, no PerFrameFunctionalGroupsSequence) through
  registerNaturalizedDatasetForLocalWadouri and assert no enumerable NumberOfFrames
  appears (and that a multiframe SEG dataset still gets frame imageIds registered).
• Status: DONE 2026-07-09 — registerNaturalizedDatasetForLocalWadouri.test.js
  (mocks dicom-image-loader + @ohif/core): SR gains no NumberOfFrames own-property;
  multiframe SEG gets NumberOfFrames = 3 (enumerable) + 3 ?frame= mappings;
  dataset.url stays non-enumerable.

[ ] 4c. (Optional, pairs with 2b) event-ordering test for the settle fix

• If 2b lands, pin the stack→volume conversion scenario: VIEWPORT_DATA_CHANGED must not
  fire before the labelmap actor exists, and a same-viewport re-entrant
  setViewportData during conversion must not interleave stale continuations. Likely an
  integration/e2e-level check; scope to what's practical.
• Status:

---

Release / dependency gating (ties into the PR-2611 plan)

• This PR is step B of the two-step rollout in
  libs/@cornerstonejs/PR-2611-review-plan.md: it hard-depends on a cs3d release
  containing fix: Use the image loader for dicom-seg to allow compressed images #2611 (createFromDicomSegImageId, prefetchPart10Instance,
  @cornerstonejs/metadata clearCacheData). That gating is handled by the CS3D_REF
  workflow + merge guard (see 1c) — merge order is: wire up CS3D_REF on the PR → CI
  green against the branch → release cs3d with fix: Use the image loader for dicom-seg to allow compressed images #2611 → switch CS3D_REF to the release
  version → cs3d-set-version.mjs + commit → merge #5806.
• The local libs/ symlink setup masks 1b on dev machines — it only reproduces on a
  fresh clone / CI installing from the registry. The CS3D_REF version-path run
  (installs from npm, no cs3d:link) doubles as exactly that link-free install check.

---

Suggested order

1. 1a, 1d — small, self-contained conformance fixes; land with 4a/4b.
2. 1b — one-hunk removal (or guard+commit); unblocks fresh clones immediately.
3. 2a, 2c — mechanical once the policy call is made (narrow registration + wire
   release; pick a race-time value).
4. 2b — needs the re-entrancy testing described; do after the mechanical items.
5. 3a — cheap insurance fix or defer with a PR note.
6. 1c — five-minute PR housekeeping (label + CS3D_REF: body line), do anytime;
   the version switch + cs3d-set-version.mjs commit happens at merge time.

[sedghi]

Looks great thanks, merge after green CI

Applied