fix(MetadataProvider): Correctly assign imageId for multiframe images and remove unused frame information retrieval method - #5965
Conversation
… and remove unused frame information retrieval method
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR updates the core metadata plumbing to better handle multiframe DICOM imageIds and simplifies frame-number handling by removing a URL-based frame parsing helper.
Changes:
- Reassign
imageIdon the returned instance fromMetadataProvider._getInstanceto avoid incorrectimageIdon combined multiframe frame instances. - Remove
getFrameInformationFromURLand switch frame derivation to rely on ingested UID mappings. - Expose the internal DICOM metadata store model on
window(new global side effect).
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| platform/core/src/services/DicomMetadataStore/DicomMetadataStore.ts | Adds a global window._model assignment (debug exposure) at module end. |
| platform/core/src/classes/MetadataProvider.ts | Adjusts multiframe instance handling and changes frameNumber derivation / removes URL frame parsing method. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Viewers
|
||||||||||||||||||||||||||||
| Project |
Viewers
|
| Branch Review |
fix/multiframe
|
| Run status |
|
| Run duration | 01m 48s |
| Commit |
|
| Committer | Alireza |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
28
|
| View all changes introduced in this branch ↗︎ | |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates multiframe image handling so combined instances keep the requested ChangesMultiframe image handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant makeDisplaySet
participant getDisplaySetInfo
participant getDynamicVolumeInfo
participant extensionManager
makeDisplaySet->>makeDisplaySet: compute display-set imageIds
makeDisplaySet->>getDisplaySetInfo: pass instances and imageIds
getDisplaySetInfo->>getDynamicVolumeInfo: request dynamic volume info
getDynamicVolumeInfo->>extensionManager: call csGetDynamicVolumeInfo(imageIds)
extensionManager-->>getDynamicVolumeInfo: return dynamic volume details
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform/core/src/classes/MetadataProvider.ts (1)
468-481: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the exact frame-specific imageId before stripping
&frame=.platform/core/src/classes/MetadataProvider.ts:468-481
DicomWebDataSourcestoresframeImageIdentries with&frame=N, so stripping the frame first skips the registered key and can resolve multiframe images to the wrong/default frame metadata. Look up the full URI first, then fall back to the frame-stripped key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform/core/src/classes/MetadataProvider.ts` around lines 468 - 481, In MetadataProvider’s image lookup flow, the current logic strips `&frame=` from `imageURI` before checking `imageURIToUIDs`, which can miss exact multiframe keys registered by `DicomWebDataSource` via `frameImageId`. Update the lookup to first query `this.imageURIToUIDs.get(imageURI)` using the full URI, then fall back to the frame-stripped URI only if needed, and keep the existing `frameNumber` handling in this same lookup block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@platform/core/src/classes/MetadataProvider.ts`:
- Around line 468-481: In MetadataProvider’s image lookup flow, the current
logic strips `&frame=` from `imageURI` before checking `imageURIToUIDs`, which
can miss exact multiframe keys registered by `DicomWebDataSource` via
`frameImageId`. Update the lookup to first query
`this.imageURIToUIDs.get(imageURI)` using the full URI, then fall back to the
frame-stripped URI only if needed, and keep the existing `frameNumber` handling
in this same lookup block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 08f96be7-b330-4fee-b1f6-90884677f2e7
📒 Files selected for processing (1)
platform/core/src/classes/MetadataProvider.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platform/core/src/classes/MetadataProvider.test.ts (1)
17-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider isolating provider state between tests.
All tests mutate the shared
metadataProvidersingleton viaaddImageIdToUIDswithout any reset (e.g.,beforeEach/afterEach). Currently safe due to unique fake URLs per test, but future additions risk cross-test pollution if URLs collide or tests are reordered/parallelized.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform/core/src/classes/MetadataProvider.test.ts` around lines 17 - 73, The MetadataProvider tests mutate the shared metadataProvider singleton via addImageIdToUIDs without clearing state, so add test isolation around each case. Update MetadataProvider.test.ts to reset or recreate metadataProvider in a beforeEach/afterEach (or equivalent) and keep the existing getUIDsFromImageID and addImageIdToUIDs assertions unchanged, so these frame-handling tests remain independent even if URLs overlap or execution order changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@platform/core/src/classes/MetadataProvider.test.ts`:
- Around line 17-73: The MetadataProvider tests mutate the shared
metadataProvider singleton via addImageIdToUIDs without clearing state, so add
test isolation around each case. Update MetadataProvider.test.ts to reset or
recreate metadataProvider in a beforeEach/afterEach (or equivalent) and keep the
existing getUIDsFromImageID and addImageIdToUIDs assertions unchanged, so these
frame-handling tests remain independent even if URLs overlap or execution order
changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cd3de9f-4aee-47ac-8fd5-1d23c78f9a13
📒 Files selected for processing (2)
platform/core/src/classes/MetadataProvider.test.tsplatform/core/src/classes/MetadataProvider.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- platform/core/src/classes/MetadataProvider.ts
| return imageURI.match(FRAME_QUERY_PARAM)?.[1]; | ||
| } | ||
|
|
||
| function removeFrameNumberFromImageURI(imageURI: string): string { |
There was a problem hiding this comment.
There are existing remove frame number versions already - lets re-use an existing one so that we can have all the removal stuff in one place.
There was a problem hiding this comment.
getUriModule(imageId) (line 17) — this is your extractor and remover in one. It returns:
baseImageId → the image id with the frame portion removed (imageId.substring(0, framesMatch.index))
frameNumber → the extracted frame as a number
framesString → the raw frame string
remaining → anything after the frame segment (query params, etc.)
| const FRAME_QUERY_PARAM = /[?&]frame=([^&#]*)/; | ||
| const FRAME_QUERY_PARAM_WITH_SEPARATOR = /([?&])frame=[^&#]*(&)?/; | ||
|
|
||
| function getFrameNumberFromImageURI(imageURI: string): string | undefined { |
There was a problem hiding this comment.
Ditto - there are already calls for this in cs3d metadata.
| // through combineFrameInstance will mistakenly get the first imageId. | ||
| // This happens because the DICOM web data store only keeps the first instance. | ||
| if (result) { | ||
| result.imageId = imageId; |
There was a problem hiding this comment.
This has to be a non-iterable getter.:
Object.defineProperty(newInstance, 'imageId', {
value: imageId,
});
Otherwise if you spread this object into another one, it will preserve the imageId, which is the wrong behaviour for things like segmentation where the object changes.
wayfarer3130
left a comment
There was a problem hiding this comment.
Just use the shared utility from cs3d for getting the frame number/extracting the base uri:
getUriModule(imageId) (line 17) — this is your extractor and remover in one. It returns:
baseImageId → the image id with the frame portion removed (imageId.substring(0, framesMatch.index))
frameNumber → the extracted frame as a number
framesString → the raw frame string
remaining → anything after the frame segment (query params, etc.)
# Conflicts: # platform/core/src/classes/MetadataProvider.ts
…define imageId as non-enumerable Addresses review feedback: frame number extraction now reuses @cornerstonejs/metadata getUriModule instead of a local regex helper, and the reassigned imageId is defined non-enumerable so spreading the instance does not carry it onto derived objects.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform/core/src/classes/MetadataProvider.ts`:
- Around line 4-12: Remove the getUriModule destructuring from
csMetadataUtilities and update getUIDsFromImageID to avoid calling
getUriModule(imageId)?.framesString; preserve the existing frameNumber sources
or use the established URI parsing logic instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 90827337-7ce4-4f53-a081-34652b8f9727
📒 Files selected for processing (1)
platform/core/src/classes/MetadataProvider.ts
Adds Playwright tests for multiframe image stacks (US cine and NM SPECT), asserting frame enumeration via the viewport overlay and actual rendered canvas content via a new getViewportCanvasStats utility, without screenshot baselines.
… and remove unused frame information retrieval method (OHIF#5965)
…mits) (#113) * fix(colors): Replace hardcoded colors with theme tokens (#6040) * docs(site): add ui-next component documentation and upgrade Docusaurus (#6102) * chore(version): Update package versions to 3.13.0-beta.106 [skip ci] * feat(ui-next): Adds appearance dialog with theme presets and custom theme support (#6041) * chore(version): Update package versions to 3.13.0-beta.107 [skip ci] * fix(ci): deploy docs with Netlify CLI monorepo filter (#6115) * chore(version): Update package versions to 3.13.0-beta.108 [skip ci] * fix(test): stabilize DICOM Tag Browser scrollbar screenshot (#6118) * chore(version): Update package versions to 3.13.0-beta.109 [skip ci] * fix(wsi): mount WSI/SM viewports via setDisplaySets (#6107) * chore(version): Update package versions to 3.13.0-beta.110 [skip ci] * fix(dicom-pdf): enhance PDF loading with authentication support - move authenticated rendered media loading into the datasource\n- route DICOM video display sets through Cornerstone video viewports\n- add a 3.12 to 3.13 migration note for the removed DICOM-video viewport namespace * chore(version): Update package versions to 3.13.0-beta.111 [skip ci] * fix: recipe config cleanup, report-only CSP, logout redirect validation (#6124) * chore(version): Update package versions to 3.13.0-beta.112 [skip ci] * fix(security): ignore decompress security vulnerability (#6125) * chore(version): Update package versions to 3.13.0-beta.113 [skip ci] * feat: Add customization URL parameter (#5992) * Add customization URL parameter * fix: Preserve should be customizeable * Update customizations docs * fix: Overlay items on patient name * Add customization test * Fix resolve to absolute path * fix: Warn on no data in load * Remove unused customization stuff * fix: PR comments * Update stored parameters to only use an array for mulitples * Remove requires ohif.* special call out * Remove strict mode * PR comments * Document segmentation examples * Add three examples as requested * PR comments * lock * Remove old customizatoin export * fix: Ordering issues on customization loads * fix: Use correct default for dev builds app config * Fixes for conflicts * chore: restore pnpm-lock.yaml to match master The lockfile diff was incidental peer-descriptor churn and carried no functional dependency change. It tripped the CircleCI security-audit gate (which only runs when pnpm-lock.yaml is in the PR diff), surfacing a pre-existing critical `decompress` transitive vuln that also exists on master. Restoring master's lockfile removes the audit trigger. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): restore json5 lockfile entry; ignore unfixable decompress GHSA The previous commit restored pnpm-lock.yaml from master, which dropped the json5@2.2.3 entry that platform/core legitimately depends on (JSONC parsing for the customization feature). That broke `--frozen-lockfile` install (ERR_PNPM_OUTDATED_LOCKFILE). This restores the correct lockfile. Because the lockfile must change (json5), the CircleCI security-audit gate runs and previously failed on a critical `decompress` <=4.2.1 zip-slip advisory. This is a pre-existing transitive vuln (present on master too) with no published patch — decompress's latest release is 4.2.1, so no version bump/override can resolve it. It reaches the tree only via @itk-wasm/dam, a build/data-asset extraction tool under @cornerstonejs/labelmap-interpolation. Add GHSA-mp2f-45pm-3cg9 to the existing pnpm-workspace.yaml auditConfig ignoreGhsas accepted-risk list, matching how the repo already exempts other build-tooling advisories. `pnpm audit --audit-level high` now passes locally (1 critical ignored, 0 high). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(e2e): fix visitStudy URL encoding that broke mpr2 study load The visitStudy rewrite (added for the ?customization= option) built the URL with new URLSearchParams({ StudyInstanceUIDs: studyInstanceUID }), which percent-encodes the value. mpr2.spec.ts embeds an extra param in the UID string ('<uid>&hangingprotocolid=mpr'), so the & and = were encoded and the whole thing collapsed into one invalid StudyInstanceUIDs value -> the study could not be found ('studies are not available'), the viewer never rendered, and the side-panel-header-right click timed out. Restore master's raw concatenation for StudyInstanceUIDs (so embedded params survive as separate query params) while still appending the customization option separately. Only mpr2 embeds & in the UID, matching the single failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * PR comments --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(version): Update package versions to 3.13.0-beta.114 [skip ci] * feat: Add extensibility for tmtv and segmentation modes * Revert "feat: Add extensibility for tmtv and segmentation modes" This reverts commit 84c7d17b1333f076188d0cc59cc6f52ae6d0da59. * chore(version): Update package versions to 3.13.0-beta.115 [skip ci] * feat: Add support for labelmap seg images in any supported tsuid (also for compressed bitmap) (#5806) * feat: Update to use pnpm (#6031) [simulated squash-merge] * feat: load DICOM SEG images via imageLoader * feat: load DICOM SEG images via imageLoader * PR review fixes * Test fragment of compressed multiframes * fix: Removed dependencies incorrectly * Update frame as a targetted change to avoid stripping remaining params * lock * Update test to agree with the missing representation branch * test(contour): contour color change coverage (#6042) * test(contour): contour interactions delete segment (#6069) --------- Co-authored-by: Bill Wallace <wayfarer3130@gmail.com> * chore(version): Update package versions to 3.13.0-beta.98 [skip ci] * feat(testing): OHIF Test Agent Skills (#5993) * chore(version): Update package versions to 3.13.0-beta.99 [skip ci] * test(contour): Add the ContourSegmentToggleLock.spec.ts test file to test contour locking (#6072) * chore(version): Update package versions to 3.13.0-beta.100 [skip ci] * chore(testing): Flock lock for playwright tests; Pin node version for OHIF; add more workers (#6099) * update Cypress apt deps for Ubuntu Noble (drop libgconf-2-4, libasound2→libasound2t64) * chore(version): Update package versions to 3.13.0-beta.101 [skip ci] * Debug fixes * perf(seg): pass explicit frame decode concurrency (16) to SEG loader Pass an explicit concurrency value (SEG_FRAME_DECODE_CONCURRENCY = 16) into createFromDicomSegImageId rather than relying on the adapter default, so the SEG frame fetch/decode parallelism is set at the call site and is ready to become configurable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(behaviours): add behaviours section + multiframe Part 10 prefetch proposal Start a "Behaviours" docs section for documenting how the system and UI behave end-to-end (observed behaviours, design proposals, and failure modes), with an index README and a Docusaurus category. First entry is the proposal for loading a multiframe SEG as a single Part 10 instance: prefetch the whole instance (gated by loadMultiframeAsPart10RaceTimeMs), parse it with dcmjs (handling multipart/related vs raw DICOM), and register the per-frame compressed pixels into the Cornerstone3D core image cache (the single uniform frame registry) so the per-frame load path is served locally while the standard decode path is unchanged. Best-effort: falls back to per-frame fetches on any failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Pre-cache the image data using a full part10 file for performance * Add customizations to specify type of segmentation save * Add clearcache of the cacheData * Bump @cornerstonejs/* pins 5.1.3 -> 5.4.10 to match libs/@cornerstonejs base libs/@cornerstonejs (fix/use-imageLoader-for-seg) is based on the released cs3d 5.4.10 (merge-base with origin/main is the 5.4.10 version bump), so pin OHIF to that release. The local branch changes still reach the app via the cs3d:link symlinks; these pins keep the lockfile/npm fallback aligned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix seg OOM * fix: file meta garbage element, bogus NumberOfFrames, unguarded source-map-loader - dicomWriter: drop the naturalized meta.TransferSyntaxUID assignment that dcmjs wrote as a garbage (0000,0000) element into every saved file's meta group (download / clipboard / local wadouri blob / local store); keep the hex 00020010 assignment, which is required for string-form _meta fallbacks. - registerNaturalizedDatasetForLocalWadouri: keep the computed frame count local so single-frame IODs (SR, RTSTRUCT) no longer gain a bogus NumberOfFrames element in their serialized form. - rsbuild.config: resolve source-map-loader opportunistically (it is not a project dependency; the rule only serves the gitignored cs3d-link workflow) so fresh clones no longer crash at config load. - Regression tests for both writer fixes (mutation-checked). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * PR review comment fixes * Update versions --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: diattamo <mmddiatta@gmail.com> Co-authored-by: ohif-bot <danny.ri.brown+ohif-bot@gmail.com> Co-authored-by: Joe Boccanfuso <109477394+jbocce@users.noreply.github.com> * fix: Several new worklist issues (#6130) * fix: Several new worklist issues * refactor: Export a single OnStudyDoubleClick type from the StudyList barrel Addresses PR review feedback: the double-click handler signature was written out in both TableProps and the WorkList customization cast, so the two could drift apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: Run worklist study double-click as a command, registrable by modes - Modes can now export getCommandsModule on their definition; appInit registers it (via the new ExtensionManager.registerCommandsModule) before the mode is instantiated, in a new 'WORKLIST' commands context, so the commands are available on the worklist before any mode route is entered. - The workList.onStudyDoubleClick customization is now a command run input (name/options) instead of a bare function, defaulting to the new launchDefaultMode command, which launches the default workflow falling back to the first applicable one. commandOptions.workflowId overrides it to a specific mode. - Duplicate mode ids are now skipped before running their modeFactory rather than after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * chore(version): Update package versions to 3.13.0-beta.116 [skip ci] * feat: Add extensibility for tmtv and segmentation modes (#6128) * feat: Add extensibility for tmtv and segmentation modes * Fixes for ordering issues on laod * Remove unnecessary reference lookup * Chane side panel timing to fix tests * PR comments - change how mode definitions get created * Improvements to mode customizations * Start organizing customizations * Misc fixes for a customization demo page * Security fixes * PR requested changes to naming * chore(version): Update package versions to 3.13.0-beta.117 [skip ci] * test(core): use fake timers in Queue unit tests to stop CI flakes (#6134) The Queue tests measured real setTimeout wall-clock time and asserted elapsed < 2 * threshold. On busy CI runners a 2400ms timer can take longer than 4800ms to fire, failing the assertion intermittently (seen in downstream validation runs). Switch to jest's modern fake timers so elapsed is exactly the timeout delay: the tests are deterministic and no longer spend ~5 seconds of real time waiting. * chore(version): Update package versions to 3.13.0-beta.118 [skip ci] * fix(MetadataProvider): Correctly assign imageId for multiframe images and remove unused frame information retrieval method (#5965) * chore(version): Update package versions to 3.13.0-beta.119 [skip ci] * feat(segmentation): replace One Click Segment with ClickSegmentTool (#6135) * feat(segmentation): replace One Click Segment with ClickSegmentTool Wire OHIF to ClickSegmentTool from Cornerstone3D (#2780), rename the toolbox button to Click to Segment, and enable it only on PET (PT) viewports. * chore(deps): bump @cornerstonejs packages to 5.4.13 Pick up ClickSegmentTool from the published Cornerstone3D release so OHIF installs the tool from npm without a local CS3D link. * chore(version): Update package versions to 3.13.0-beta.120 [skip ci] * fix(app): appearance modal provider scope, worklist preview persistence, and tag browser label overflow (#6136) - Insert ServiceProvidersManager providers ahead of the dialog/modal providers in App.tsx: modal content renders as a sibling of the provider's children, so contexts registered via the manager (e.g. ActiveThemeProvider) were out of scope and the appearance modal crashed with 'useActiveTheme must be used within an ActiveThemeProvider'. - Persist the worklist preview panel open/closed state in sessionStorage so it survives navigating into a study and back. - Keep the DICOM tag browser instance number label on one line: the words truncate, the (n of total) digits never clip. * chore(version): Update package versions to 3.13.0-beta.121 [skip ci] * test: add E2E test for contour combine intersect and subtract operations (#6131) * chore(version): Update package versions to 3.13.0-beta.122 [skip ci] * feat(next): native Generic Viewport migration (useNextViewports) (#6101) * feat(core): add appConfig.useNextViewports flag (Generic Viewport M0 step 1) Opt-in flag to drive viewports through the DIRECT native cornerstone3D GenericViewport ("next") API surface (PLANAR_NEXT / VOLUME_3D_NEXT, setDisplaySets, setDisplaySetPresentation, setViewState, view references) instead of the legacy stack/volume methods. Distinct from (and overrides) useGenericViewport, which only routes legacy viewport types through cornerstone compatibility adapters. This flag does NOT set cornerstone rendering.useGenericViewport; it is read by getCornerstoneViewportType and the CornerstoneViewportService backend split (subsequent M0 steps). Defaults false; the legacy path stays byte-identical. Opt-in only. * feat(cornerstone): map viewport types to native *_NEXT under useNextViewports (M0 step 2) getCornerstoneViewportType gains an optional useNextViewports param. When set, stack/volume/orthographic collapse to PLANAR_NEXT (render path inferred from data shape), and volume3d/video/wholeslide/ecg map to their VOLUME_3D_NEXT / VIDEO_NEXT / WHOLE_SLIDE_NEXT / ECG_NEXT types. Defaults false → legacy mapping byte-identical; no caller passes true yet (wired via appConfig in the service split, step 3). Tests: +6 cases (21 total) covering the *_NEXT mapping, displaySet override, the invalid-type throw, and that the legacy mapping is unchanged when the flag is off. * feat(cornerstone): native-next stack mount behind useNextViewports (M0 step 3 foundation) Wires the useNextViewports flag through and mounts stack viewports natively: - nextViewports.ts: module accessor; init.tsx captures appConfig.useNextViewports. - getCornerstoneViewportType: defaults the flag from the accessor, and now passes native (*_NEXT) types through idempotently (a viewport's stored cs type is re-fed into the mapper; legacy types were already idempotent, native were not). - CornerstoneViewportService._setDisplaySets: route native generic viewports by data shape (StackData vs VolumeData), since PLANAR_NEXT is one type for both. - _setStackViewport: native branch mounts via genericViewportDataSetMetadataProvider.add + setDisplaySets, applies VOI/colormap via setDisplaySetPresentation and displayArea/rotation/flip via setViewState (no legacy setStack/setProperties/setCamera). Validated in a running OHIF (linked cornerstone 5.0.8): with the flag on, the viewer creates a native PlanarViewport (window.cornerstone...getViewports()[0] -> 'PlanarViewport :: type=planarNext'); the prior 'Invalid viewport type: planarNext' is resolved. Flag OFF is byte-identical (native branches gated by isGenericViewport). KNOWN WIP (next): flag-ON full render is blocked by the presentation-read seam — peripheral consumers (useViewportRendering, overlays, colorbar, resize) still call legacy getProperties/getViewPresentation/getCamera on the viewport. Stack render completes once those are routed through getDisplaySetPresentation/viewportProjection. * feat(cornerstone): render native Generic (next) stack viewport behind useNextViewports Makes the flag-on native PLANAR_NEXT stack render end-to-end in OHIF: - CornerstoneCacheService: resolve the stack-vs-volume data-builder from the legacy mapping, since native types collapse that distinction into PLANAR_NEXT. Without this the stack fell through to the 'other' builder and imageIds were never populated (PlanarViewport threw 'No registered planar dataset metadata'). - ImageOverlayViewerTool: skip overlay rendering when the viewport has no resolvable view reference yet (native returns falsy until data is bound), instead of letting getTargetId() throw and kill the route during enable. - Add getViewportPresentation helpers (getViewportProperties / getViewportCameraState) bridging legacy getProperties/getCamera and native getDisplaySetPresentation/ getViewState; apply at the toolbar property evaluator, VOI-range init, and the position/LUT presentation snapshots so reads no longer throw on native viewports. Validated in a running OHIF (flag on): PlanarViewport(planarNext), 295 slices, image renders, zero console errors, scroll + setImageIdIndex navigate correctly. Legacy (flag off) behavior is unchanged. Native volume/MPR is a later increment. * feat(cornerstone): render native Generic (next) volume/MPR behind useNextViewports Mounts volumes on a direct PLANAR_NEXT viewport for volume/MPR rendering: - CornerstoneViewportService.setVolumesForViewport: native branch -> new _setNativeVolumeDisplaySets. Each base volume is registered with its already-cached volumeId and bound via setDisplaySets at the viewport's orientation (first = source, others = overlay); VOI/colormap/invert applied per-binding via setDisplaySetPresentation(dataId, props). Skips the legacy setVolumes/setProperties/setPresentations surface a PLANAR_NEXT viewport does not expose. Cornerstone reuses the OHIF-cached volume (getVolumeId returns the passed volumeId) and selects the image vs reformatted-volume render path from the requested orientation. - useViewportRendering colormap resolver: read via getViewportProperties for native viewports (getDisplaySetPresentation) instead of getProperties/getActors, which threw 'Error getting viewport colormap' on native volume. Validated in a running OHIF (flag on): axial/sagittal/coronal all render in volume mode, scroll navigates the volume, round-trip volume<->stack switches getCurrentMode cleanly, zero console errors. Legacy (flag off) unchanged. * feat(cornerstone): allow setViewportOrientation on native volume viewports The setViewportOrientation command guarded on isOrthographicViewportType, which is false for a native PLANAR_NEXT viewport (it reports type=planarNext even when rendering MPR). Add the content-mode capability guard (csUtils.viewportIsInVolumeMode) so the MPR orientation toolbar works on native viewports; PlanarViewport.setOrientation already exists. Legacy behavior is unchanged (viewportIsInVolumeMode is false for legacy viewports, so the existing isOrthographicViewportType branch still gates them). Also flips useNextViewports on in the dev default config for local testing of the native path (NOT for merge; see TODO_BEFORE_MERGE). * feat: Add temporary support for native GenericViewport ("next") migration - Introduced a dev-only configuration flag to toggle the native viewport backend. - Added a toolbar button to switch between legacy and native viewports for debugging. - Implemented logic to handle image slice data and viewport type detection for native viewports. - Enhanced viewport service to derive default VOI window/level from DICOM metadata. - Added utility functions for managing localStorage overrides for viewport settings. - Marked all temporary changes with comments for easy identification and removal before merging. * Add plan viewer HTML page for migration master plan display * feat(cornerstone): fork viewport presentation read/write into the backend (§4.3) Extends the legacyBackend/nextBackend seam so presentation read/write is forked, not inline-guarded in the service: - IViewportBackend gains getPositionPresentation / setPositionPresentation / setLutPresentation. LegacyViewportBackend keeps the exact legacy logic (getViewPresentation / setProperties / setViewPresentation) byte-identical; NextViewportBackend uses the native surface (getViewReference + setViewReference; setDisplaySetPresentation for VOI/colormap/invert). WITH_ORIENTATION is inlined in the backends to avoid a backend->service value-import cycle. - CornerstoneViewportService._getPositionPresentation / _setLutPresentation / _setPositionPresentation now delegate to this.backend. _getLutPresentation stays shared (already native-aware via the getViewportProperties bridge). Effect: setPresentations is now native-safe — it previously threw on a PLANAR_NEXT viewport (setProperties / setViewPresentation), so the native mount skipped it; the native path now round-trips presentation cleanly. Native pan/zoom persistence (viewPresentation is still undefined on native) is a later increment. Validated both lanes: native PlanarViewport renders + storePresentation/getPresentations/ setPresentations round-trip with no crash; legacy StackViewport byte-identical round-trip; zero console errors each. * feat(cornerstone): persist native viewport pan/zoom/rotation/flip via the backend A PLANAR_NEXT viewport has no getViewPresentation/setViewPresentation, so pan/zoom previously did not survive navigation/resize/layout on the native path. NextViewportBackend now snapshots the pan/zoom subset of the semantic view state and restores it: - getPositionPresentation: snapshots the PlanarViewState pan/zoom fields (displayArea, anchorWorld, anchorCanvas, scale, scaleMode, rotation, flipHorizontal, flipVertical) via getViewState() (already deep-cloned + serializable), stored in viewPresentation. Slice and orientation are intentionally excluded (they ride on the view reference). - setPositionPresentation: applies the view reference first (slice/orientation), then a partial setViewState patch with only the pan/zoom subset — the merge preserves slice/orientation, so the view reference is never clobbered. Stale displayArea is cleared when live anchor/scale pan/zoom is restored. anchorCanvas is canvas-fractional, so it survives resize without drift. - _setStackViewport native branch now restores the persisted positionPresentation on mount (position-only; LUT already applied inline), so a returning stack recovers its camera. Validated on native: zoom -> snapshot (scale captured) -> reset -> restore (scale back), slice unchanged, zero errors. Legacy path unchanged. Volume/MPR mount pan/zoom restore is a follow-up (its native mount helper does not yet thread presentations). * feat(cornerstone): restore native volume/MPR pan/zoom on mount Extends native pan/zoom persistence to the volume/MPR mount: the native branch of setVolumesForViewport now restores the persisted positionPresentation (view reference + pan/zoom via the backend) after _setNativeVolumeDisplaySets, mirroring the stack mount. Position-only (LUT applied per-binding above), native-safe. Validated on a native sagittal MPR: zoom -> snapshot (scale 1.7) -> reset -> restore (scale back to 1.7) with orientation (sagittal) and slice (256) preserved; zero errors. * fix(next): bridge invert/flip/window-level commands for native viewports Add setViewportProperties/setViewportCameraState write bridges alongside the existing read bridges, and route invertViewport, flipViewportHorizontal, flipViewportVertical and setViewportWindowLevel through them. Direct PLANAR_NEXT viewports have no getCamera/setCamera/getProperties/ setProperties, so these four commands threw on native. The bridges dispatch on isGenericViewport: native reads/writes via getViewState/setViewState (flip) and getDisplaySetPresentation/setDisplaySetPresentation (invert/voiRange) on the active binding; legacy falls through to the identical getCamera/setCamera/ getProperties/setProperties calls, so flag-off stays byte-identical. Verified live on a native stack viewport: all four now apply (invert undefined->true, flipH/flipV false->true, voiRange retargets) instead of throwing. * fix(next): apply setViewportColormap on native viewports setViewportColormap was fully guarded by isStackViewportType/ isOrthographicViewportType, both of which report false for native PLANAR_NEXT viewports, so the command was a silent no-op on native (returned ok but applied nothing). Add a native branch that applies the colormap via the setViewportProperties bridge (setDisplaySetPresentation) on the active binding, honoring the immediate render flag, before the legacy guards. Verified live: HSV colormap now applies and renders on a native stack viewport. * fix(next): make ColorbarService native-safe ColorbarService called getActors/getProperties/setProperties directly, all of which throw on direct PLANAR_NEXT (next) viewports, so toggling a colorbar threw on native. Replace the getActors content gate with a viewportHasContent helper (getCurrentMode for native, getActors for legacy), and route the property read/write through the getViewportProperties/setViewportProperties bridges. Legacy keeps the identical getActors/getProperties/setProperties calls. Verified live on native: addColorbar no longer throws, hasColorbar becomes true, and the colormap applies via setDisplaySetPresentation. * fix(next): guard per-volume histogram WL panel for native viewports getWindowLevelsData drives the per-volume histogram WL panel via getAllVolumeIds/getProperties, which direct PLANAR_NEXT (next) viewports do not expose (they throw). Add an early guard that returns no rows when getAllVolumeIds is absent, so the panel degrades to 'No window level data available' instead of erroring on the interval/event refresh. Legacy stack viewports also lack getAllVolumeIds and were never passed here, so this is a no-op for them; the native stack/volume WL path is driven by setViewportWindowLevel. * fix(next): make resetViewport/scaleViewport/rotate commands native-safe These three command paths assumed legacy camera APIs that direct PLANAR_NEXT (next) viewports do not expose: - resetViewport called viewport.resetCamera() (absent on native -> threw). Native branch uses resetViewState() (resets pan/zoom/rotation/orientation/flip; navigation/slice preserved). resetProperties stays optional-chained. - scaleViewport (scaleUpViewport/scaleDownViewport) was guarded by isStackViewportType, which is false for native, so the zoom buttons were a silent no-op. Native branch uses getZoom/setZoom; parallelScale and zoom are inversely related so it divides by scaleFactor to match legacy direction. - _rotateViewport (rotateViewportCW/CCW/CWSet) used getViewPresentation/ setViewPresentation (absent on native). Native branch reads rotation/flip via the getViewportCameraState bridge (getViewState) and writes the new rotation via setViewportCameraState (setViewState), preserving the flip-parity logic of the 'set' mode. Verified live on native stack: reset no longer throws and resets zoom; zoom in 1->1.111; CW/CCW/Set rotation applies (90/180/90/90). Legacy unchanged. * fix(next): guard jumpToMeasurement camera-centering for native viewports jumpToMeasurement re-centers the camera when a measurement is off-screen via isMeasurementWithinViewport (calls viewport.getCamera()) + getCamera/setCamera. Native PLANAR_NEXT viewports have neither, so jumping to a measurement threw 'viewport.getCamera is not a function' at the gate before the centering block. Short-circuit the centering on native (isGenericViewport) so it is skipped; setViewReference above already navigated to the measurement's slice, so the measurement is still reached - only in-plane re-centering is deferred. TODO(next): port in-plane centering via the camera bridge + setViewState pan. Verified live: native viewport has no getCamera (getCamera() throws) and isGenericViewport is true, so the throwing branch is now skipped; setViewReference remains available for slice navigation. Legacy unchanged. * fix(next): make getViewportAlignmentData + updateViewport native-safe Two CornerstoneViewportService methods called getCamera()/setCamera(), which direct PLANAR_NEXT (next) viewports lack: - getViewportAlignmentData looped every viewport reading getCamera().viewPlaneNormal (reached from findNavigationCompatibleViewportId on a cross-orientation jumpToMeasurement). Native reads viewPlaneNormal from getViewReference() instead (both backends populate it); legacy keeps getCamera (byte-identical). - updateViewport (metadata-invalidation re-mount, keepCamera) read getCamera() unconditionally and had no native branch in its stack/volume if/else, so it threw and would not re-mount native data. Add a native branch that snapshots/restores the camera via the view-state bridges and routes the re-mount through _setDisplaySets (backend.dispatchMount, which dispatches by data shape). Legacy path unchanged. Verified live on native: getViewportAlignmentData returns data (no throw); updateViewport(keepCamera) re-mounts, restores zoom (1.4/1.5), keeps the image actor and renders, with zero errors. * refactor(next): move viewport interaction ops into a Legacy/Next operations backend The commandsModule carried inline native-vs-legacy branches for every viewport interaction/appearance command. Extract them into a dedicated operations backend that mirrors the existing IViewportBackend twin pattern: - IViewportOperations: the interface (flip/invert/rotate/reset/scaleBy/ setWindowLevel/setColormap/getViewPlaneNormal/centerOnMeasurement + 3D VR ops) - LegacyViewportOperations: legacy lane via direct legacy APIs (getCamera/ setProperties/getViewPresentation/resetCamera/actors), lifted verbatim - NextViewportOperations: native lane via the presentation/camera-state bridges + native semantic API (getViewState/resetViewState/getViewReference/setZoom); the 3D VR ops warn-once and no-op behind a CS-14 gate (native VR not supported yet) - viewportOperations: per-viewport dispatcher (isGenericViewport ? next : legacy) commandsModule's 13 interaction commands become one-line delegations (the file shrinks ~239 lines) and CornerstoneViewportService.getViewportAlignmentData uses viewportOperations.getViewPlaneNormal. Dispatch is per-viewport (not flag-selected like the lifecycle IViewportBackend) because operations run on already-created, self-describing viewports and a session can mix lanes; this preserves the previous inline isGenericViewport branching exactly. Render() stays in the command (per-command render timing preserved). Validated live both lanes: native (flag on) applies all ops (invert/flip/rotate/ zoom/WL/colormap; reset is camera-only) and legacy (flag off) is byte-identical (parallelScale*0.9 zoom, getViewPresentation rotation, resetProperties+resetCamera), both with a clean console. Adversarial review found no byte-identity/runtime defects. Segmentation untouched. * feat(next): render native 3D volume rendering + enable its VR operations Make native VOLUME_3D_NEXT viewports actually render volume rendering and wire up the 3D VR operations: - CornerstoneViewportService._setNativeVolumeDisplaySets: a 3D viewport (cornerstone type VOLUME_3D_NEXT) now mounts with setDisplaySets({ options: { renderMode: 'vtkVolume3d' } }) instead of the planar { orientation, role }, and applies the display-set's volume-rendering preset to the volume actor via csUtils.applyPreset (the bare native VolumeViewport3D has no setProperties). colormap (a planar LUT concept) is skipped for 3D. The dataId registration is unchanged (the volume3d data provider reads imageIds/volumeId and ignores the stored kind). - NextViewportOperations: the four VR ops are no longer CS-14 no-ops. setPreset applies the preset to the volume actor via applyPreset; setVolumeRenderingQuality/ shiftVolumeOpacityPoints/setVolumeLighting operate on the vtk volume actor through getActors (which native VolumeViewport3D exposes), so they reuse the legacy actor-based implementations. Pairs with the cornerstone fix that keeps the 3D viewport's canvas visible. Verified live: native 3D VR renders (CT-Bone/CT-Cardiac presets) and all four VR commands apply on a native VOLUME_3D_NEXT viewport; legacy VOLUME_3D unchanged. * docs(next): refresh migration plan to HEAD (2026-06-19 audit) * fix(next): target fusion colormap at the overlay binding NextViewportOperations.setColormap dropped params.displaySetInstanceUID and always wrote to the active source binding via getSourceDataId(), so a PT/CT fusion colormap landed on the CT source instead of the PT overlay. Thread the displaySetInstanceUID through to setViewportProperties so it targets the right native binding (OHIF maps each display set 1:1 onto its bare dataId); falls back to the source when no id is given (single-volume / plain stack colormap). Also document DataIdRegistry.dataIdFor's 'overlay' suffix as reserved for the same-UID source/overlay case (derived labelmap overlays, M4) rather than fusion, whose distinct-UID overlays are already collision-free under the bare id. * fix(next): make residual native-unsafe viewport sites safe Sweeps the OHIF-side sites a native PLANAR_NEXT viewport reaches that still called legacy-only APIs (getProperties/getCamera) or branched on the collapsed viewportType: - CornerstoneCacheService: persist the legacy stack/volume decision as viewportData.dataShapeType (createViewportData) and branch on it in invalidateViewportData instead of viewportType. Native collapses stack+volume onto PLANAR_NEXT, so a native stack previously fell through to the VOLUME rebuild and re-mounted as volume data on metadata invalidation. Falls back to viewportType for legacy/older data (byte-identical off-path). - ViewportOrientationMarkers: gate the synthetic-IOP default-cosine check on dataShapeType, not viewportType==='stack' (dead on native -> guard was skipped). - CornerstoneViewportDownloadForm: the capture viewport is the source's type, so a native source threw on getProperties/setStack/setProperties. Add a native capture path that re-mounts the source's already-registered dataId via setDisplaySets and copies presentation + view state through the bridges (legacy path byte-identical). - tmtv ROI-threshold: read the slice focal point via a new getViewportFocalPoint bridge (native getViewReference().cameraFocalPoint vs legacy getCamera().focalPoint) instead of getCamera(), which is absent on native. New bridge getViewportFocalPoint added to getViewportPresentation.ts and exported from @ohif/extension-cornerstone. Validated live: native stack renders, console clean, viewportData.dataShapeType='stack' while viewportType='planarNext', orientation markers render. * feat(next): mount native video/WSI/ECG viewports Under useNextViewports, NextViewportBackend.dispatchMount routed ALL viewports by data shape (volume vs stack), so VIDEO_NEXT/WHOLE_SLIDE_NEXT/ECG_NEXT constructed as native classes but mis-ran _setStackViewport's stack-specific prefetch/VOI/ kind:'planar' logic and never reached their dedicated mounts; ECG additionally called the absent setEcg. - NextViewportBackend.dispatchMount: route the non-planar families by viewport type to _setEcgViewport / _setOtherViewport (mirrors the legacy backend's type dispatch); planar stack/volume still routes by data shape. - _setEcgViewport: native branch registers {kind:'ecg', sourceDataId} and mounts via the generic setDisplaySets API (native ECG has no setEcg). - _setOtherViewport: native branch registers {kind:'video', sourceDataId} or, for WSI, {kind:'wsi', imageIds, options:{webClient}} with the client resolved from WADO_WEB_CLIENT metadata exactly as the legacy WSI adapter does, then mounts via setDisplaySets + setViewReference. - DataIdPayload widened to a family-specific union (planar/video/ecg/wsi). All registration goes through the ref-counted DataIdRegistry (§4.7). OHIF-only — the cornerstone native classes already support setDisplaySets (per the genericVideo/genericEcg/genericWsi examples). Validated: native planar render unaffected by the dispatch change (console clean); video/WSI/ECG mounts follow the canonical cornerstone examples but are not yet live-validated (no such study on the dev dicomweb). * chore(next): guard the useNextViewports flag-read allowlist (M7 prep) Adds .scripts/check-next-viewports-flag-reads.mjs (wired as `yarn next:check-flag-reads`) enforcing migration plan §4.2: the flag may be read only in the sanctioned seam — getCornerstoneViewportType (type selection), CornerstoneViewportService (backend selection), nextViewports.ts (the accessor), init.tsx (the one appConfig.useNextViewports read), and the TEMP dev toggle in getToolbarModule.tsx. Any other isNextViewportsEnabled()/appConfig.useNextViewports read under extensions/cornerstone/src fails the check, so the legacy off-path cannot drift. (Comment-only mentions are ignored; tests are exempt.) The earlier audit framed the '2 sanctioned reads' contract as already violated by a 'backend trio', but those files only MENTION the flag in doc comments — the actual runtime read surface is the sanctioned set above, so the rule is enforceable as written. TODO_BEFORE_MERGE.md updated: the guard is permanent (not a dev revert), and removing the dev toggle must also drop its allowlist entry. Does NOT perform the destructive M7 reverts (config default flip, toggle button): those are premature while segmentation/M4 is unmigrated and would disable the in-browser test loop. * docs(next): mark CS-12 native calibration done; refresh CS-20/M6 status * docs(next): re-verify migration status at HEAD; correct stale prose Re-audited every milestone (M0-M7) and CS blocker against HEAD source via a multi-agent audit + adversarial verification pass. Five commits landed after the last full prose refresh (05e0df0ca) and only d5d03d888 touched the doc, so the Implementation status section was materially stale. Corrections: - M2: fusion colormap keying is FIXED (7b61e08ee), not 'unsound' - M3: four 'native-unsafe throws' are FIXED (a19bd7826); the per-volume WL panel is guarded (d28202610), not throwing - feature port, not a crash - M6: video/WSI/ECG ARE mounted natively (ca746e2f0) - M7: flag-read allowlist IS built (b5784ca80), just not wired into CI - CS-21: stated trigger is unreachable; narrowed to single-point SCOORD3D, and the open code is PlanarViewReferenceController.ts (not planarViewReference.ts) Adds a consolidated, verified remaining-work punch-list and a corrections subsection. The one real native crash that remains is the M4 segmentation OHIF half (convertStackToVolumeViewport throws AND promotes to legacy ORTHOGRAPHIC). * Refactor SegmentationService to support dual backends for segmentation handling - Introduced ISegmentationBackend interface to define methods for segmentation backends. - Implemented LegacySegmentationBackend for existing behavior with stack/volume promotion. - Implemented NextSegmentationBackend for native GenericViewport handling without promotion. - Updated SegmentationService to utilize the appropriate backend based on viewport type. - Removed legacy viewport handling logic from SegmentationService and delegated to backends. - Enhanced segmentation data assembly to support overlapping segments in the Next backend. - Updated CornerstoneViewportService to ensure proper restoration of segmentation presentations. - Added support for preserving additional query parameters in the application. * temp * Refactor CornerstoneViewportService to optimize setDisplaySets handling for 3D volumes * d * Refactor viewport handling and opacity management for native volume rendering * fix(WindowLevel): re-sync fusion tab to foreground default after async resolve The effect only adopted the foreground (PT) default when activeDisplaySetUID was falsy, so if foregroundDisplaySets was empty at mount the tab seeded to the CT fallback and stayed pinned to CT once PT resolved. Track explicit user selection and re-sync to the foreground default until the user picks a tab. * chore(next): remove WIP migration plan artifacts from the branch Delete the planning docs and plan-viewer pages that were committed during development (migration plans, blueprint, TODO_BEFORE_MERGE, plan-viewer.html). They are not part of the shipped viewer and only attract review noise. * fix(next): address review findings (guards + correctness) Apply CodeRabbit review comments on the migration code: - commandsModule: guard missing viewport before setWindowLevel/render - WindowLevel: drop stale activeDisplaySetUID when the viewport's display sets change - SegmentationService: hard guard when segmentation lookup fails before backend classify - LegacyViewportOperations: guard getActors()[0] before actor-chain calls - CornerstoneViewportService: guard empty native stack imageIds; don't route generic overlay-only mounts to legacy setVolumes; stop overwriting tracked display sets with base-volume-only ids (drops SEG/RT/fusion overlay UIDs) - getCornerstoneViewportType: list orthographic/volume3d in the invalid-type error - nextViewports: skip reload (warn) when the toggle can't be persisted - tmtv: guard missing focal point before mutating ROI annotation coordinates - check-next-viewports-flag-reads: also catch bracket/destructured flag reads * fix(overlay): show instance number on next viewports The viewport overlay's getInstanceNumber switched on viewportData.viewportType, which for next viewports is the native PLANAR_NEXT type (the stack/volume shape is persisted separately as dataShapeType). The switch matched no case, so the instance number was null and the overlay showed only the slice index/count. Switch on (dataShapeType ?? viewportType) so next stack/volume viewports take the correct branch (legacy is unaffected). Also make _getInstanceNumberFromVolume read the view-plane normal via getViewReference for native viewports, which expose no getCamera, so routing next volume viewports through it cannot throw. * fix(overlay): refresh window level on series change for next viewports The overlay's WW/WL comes from useViewportRendering, whose init effect reads the VOI via getViewportProperties. On series change the effect re-runs, but native (next) viewports expose only explicit VOI overrides through getDisplaySetPresentation; a freshly shown series has none, so properties.voiRange was undefined, setVoiRange was skipped, and the overlay kept showing the previous series' window level. Legacy getProperties always returns the applied VOI, so only native viewports were affected. Fall back to the viewport's computed default VOI (getDefaultVOIRange) for generic viewports when no override is stored, matching the LivewireContourTool and WindowLevelTool bridges. Legacy behavior is unchanged. * fix(scrollbar): seed slice state on orientation change for next viewports The progress scrollbar seeded its slice state (imageIndex/numberOfSlices) only when viewportData changed. Native (next) viewports keep the same viewportData across a stack->volume transition or an orientation change, and the slice- navigation event does not fire until the first scroll, so the scrollbar was missing on the initial slice (or stale with a wrong slice count) until the user scrolled once. Re-seed the slice state from the live viewport on CAMERA_MODIFIED (which native viewports emit on orientation/geometry changes, as the sibling full-mode hook already relies on). A guard skips redundant state updates so pure pan/zoom does not churn React state. * refactor(next): call resetDisplaySetPresentation on reset Follow the cs3D rename of the native viewport's presentation-reset method from resetProperties to resetDisplaySetPresentation (the next viewport API uses get/set DisplaySetPresentation, not get/set Properties). Behavior unchanged. * fix(segmentation): make border/outline thickness slider integer-only The Border (outline width) slider for labelmap and RTSTRUCT used step=0.1, allowing fractional outline widths. Outline thickness is a pixel width and should be a whole number, so use step=1 and round the committed value. The fill/opacity sliders keep their fractional step. * fix(crosshairs): guard resetCrosshairs against unregistered Crosshairs tool resetCrosshairs (run by Reset Viewport) called toolGroup.getToolInstance('Crosshairs') for every tool group; getToolInstance logs 'Crosshairs is not registered with this toolGroup' when the tool is absent, and a next viewport's default tool group does not include Crosshairs, so Reset Viewport logged a spurious warning. Guard the lookup with toolGroup.hasTool('Crosshairs') and skip tool groups that lack it; also guard against a missing tool group for the viewport. * fix(next-fusion): promote source to volume slice when a data overlay is added A data overlay (fusion) on a next (PLANAR_NEXT) viewport rendered the source as a vtkImage stack while the overlay was a vtkVolumeSlice, producing a broken/unstable fusion (geometry mismatch, intermittent across slices/scroll). Two causes: 1. CornerstoneCacheService.createViewportData built stack data when the fusion's primary display set resolved to a stack shape, so the source never got a volumeId. Force a volume (orthographic) shape when there are 2+ reconstructable image display sets (a data fusion must be volume; legacy already did this, and non-reconstructable SEG/RT overlays are excluded). 2. dataIdRegistry.register used first-writer-wins, so re-registering the source (originally a vtkImage stack, no volumeId) with its fusion volumeId was dropped, leaving its dataset volumeId-less -> the render-path decision kept it vtkImage. Update the provider when a payload promotes a dataId to volume-backed. Validated via agent-browser: adding a PT overlay onto a CT source now mounts both as vtkVolumeSlice (mode=volume), the fusion is anatomically coherent and stable across scroll, with no console errors. * fix(next-rtss): keep referenced CT in stack mode on RTSTRUCT hydrate RTSTRUCT (contour) hydration on a native PLANAR_NEXT viewport re-mounted the referenced CT as a volume slice, which is the slow path the perf AC forbids. Spike proved cs3d already renders contour segmentations on a stack/vtkImage PLANAR_NEXT viewport (via the annotation + isReferenceViewable path), and that stack-mode contour scroll is fast (~0.15ms/scroll, no metadata storm) once the canvas-dimension layout-thrash fix is in place. The only remaining issue was the hydrate re-mount promoting the CT to volume. Pin the referenced viewport to 'stack' for RTSTRUCT hydration on next viewports: hydrateSecondaryDisplaySet passes viewportType:'stack' (scoped to RTSTRUCT + isNextViewportsEnabled), and loadSegmentationDisplaySetsForViewport applies it as a per-mount viewportOptions override. SEG and legacy keep their current behavior. Verified at runtime: after hydrate the viewport stays vtkImage/stack, contours render across slices, scroll is 0.16ms with 0 metaData.get calls, CT VOI intact. * fix(next-fusion): match legacy initial data-overlay opacity (~40%) on next viewports The data-overlay add path passes a nominal colormap opacity of 0.9. Legacy volume rendering attenuates that to ~40% effective via ray-cast opacity-unit- distance correction, but native PLANAR_NEXT viewports composite the overlay as a flat 2D image-slice blend with no such attenuation, so the same 0.9 rendered at ~80-90%. Override the initial overlay opacity to 0.4 for next viewports (mirrors the TMTV fusion NEXT_FUSION_PT_OPACITY), gated on isGenericViewport and a numeric opacity so SEG/RTSTRUCT overlays and the legacy path are unaffected. * fix(next-fusion): preserve fusion on orientation change in next viewports The orientation corner menu branched on viewportType === ORTHOGRAPHIC to decide in-place reorient vs viewport recreation. Native next viewports always report planarNext, so a fusion already in volume mode wrongly took the recreation path, which passes empty displaySetOptions and drops the PET overlay colormap/opacity (rendering PET only). Branch instead on whether the live viewport is already in volume mode (isOrthographicViewportType || utilities.viewportIsInVolumeMode): volume-mode viewports reorient in place (setViewportOrientation, which preserves all bindings and their presentation); only a genuine stack->volume conversion recreates. Legacy ortho/stack behavior is unchanged. * fix(next-seg): preserve base image window level through SEG hydration Hydrating a SEG re-mounted the referenced image and restored a stale, computed VOI from the LUT presentation store, brightening the base image (e.g. an MR went from its DICOM WC/WW default to a volume min/max default). During the SEG-load intermediate mount the base image briefly carries a computed default VOI; the native read bridge returned it with no isComputedVOI marker, so cleanProperties never stripped it and it was persisted then replayed over the correct default. Legacy StackViewport tags computed VOIs (isComputedVOI) so they are stripped. Mirror that: in getViewportProperties, stamp isComputedVOI on a native binding's VOI when it matches the binding's getDefaultVOIRange, so the LUT capture strips it. Harmless when a genuine user VOI equals the default (stripping falls back to the same value). * fix(next-mpr): re-seed slice scrollbar after post-mount camera carry On a stack->volume/MPR transition the slice carry (e.g. layout-selector MPR HP restoring the prior slice) moves the camera and fires its slice events synchronously during the mount, before the scrollbar effect attaches its listeners and around its initial seed -- so the scrollbar latched the mount-time index. Re-seed once on the next frame after mount+carry settle; the pushSliceData guard makes it a no-op when nothing changed (no churn). * chore(deps): bump @cornerstonejs/* to 5.1.2 * chore: empty commit * fix(next): use published genericViewportDisplaySetMetadataProvider export The next viewport backend imported genericViewportDataSetMetadataProvider from @cornerstonejs/core, a symbol that only existed in the local custom cornerstone worktree (linked via symlink). Published cornerstone exports it as genericViewportDisplaySetMetadataProvider (same add/remove/get/clear API). CI always builds against published packages, so the production rspack build failed (ESModulesLinkingError -> Netlify red) and the cypress dev-server build showed a full-screen error overlay that blocked all clicks (PR_CHECKS red). Renaming to the published symbol fixes both. * test(e2e): extend Scoord3dProbe jump screenshot retry window The jump-to-measurement screenshot was the only failing assertion (pre/post hydration pass). It uses waitVolumeLoad:false, so on slower CI the dynamic tfl_dyn_fast_tra series can still be progressively loading when the shot is taken, giving a partial probe value (52.0 vs baseline 78.0) and shifted VOI. Raise checkForScreenshot attempts 10->20 and delay 1250->2000ms (~11s -> ~40s) to let the volume settle before failing. * fix(next): address review findings (scaleBy 3D crash, fusion W/L target, off-path gate, seg export perf) - NextViewportOperations.scaleBy: guard getZoom/setZoom so the zoom hotkey no-ops on a native VolumeViewport3D instead of throwing (matches legacy). - Native setWindowLevel: forward displaySetInstanceUID so PT/CT fusion W/L targets the intended binding (mirrors setColormap) instead of always source. - CornerstoneCacheService: scope the reconstructable-fusion STACK->ORTHOGRAPHIC promotion to PLANAR_NEXT so the legacy (flag-off) path stays byte-identical. - dicom-seg buildLabelmap3D: precompute a referencedImageId->index Map to drop the multi-layer export from O(slices^2) to O(slices). * fix(next): address CodeRabbit review (dispatch discriminator, seg return contract, error msg) - NextViewportBackend.dispatchMount: route stack/volume on the persisted dataShapeType contract instead of the lazily-populated 'volume' field probe. - SegmentationService.attemptStackToVolumeConversion: return false explicitly on the frame-of-reference-mismatch path to honor the Promise<boolean> contract. - getCornerstoneViewportType: list orthographic/volume3d in the legacy-path invalid-type error (both are valid and handled above the throw). * test(next): update getCornerstoneViewportType invalid-type assertion Match the legacy-path error message now listing orthographic/volume3d (b16129ece). * chore(next): replace dev toggle + flag-read guard with URL opt-in - Remove the .scripts/check-next-viewports-flag-reads.mjs CI guard and its package.json script entry. - Drop the TEMP ToggleNextViewport toolbar button across all modes plus the toggleNextViewports command and getToolbarModule evaluator (revert mode files to their master state; keep the real native-path toolbar fixes). - Remove the localStorage override / toggleNextViewportsAndReload from nextViewports.ts; resolveNextViewportsEnabled now honors a ?useNextViewports URL query param (true/1/empty enable) over appConfig. - Stop forcing useNextViewports:true in default.js (opt-in via URL/appConfig). - Preserve useNextViewports across navigation (preserveQueryParameters). * fix(tmtv): keep legacy fusion PT opacity ramp; flatten only on next path Flattening the fusion PT opacity to a scalar 0.9 in hpViewports changed the legacy TMTV fusion rendering (the ramp keeps low PT values transparent so the CT shows through). Restore the legacy ramp in hpViewports and instead replace it with the flat native scalar (0.4) inside getHangingProtocolModule only when useNextViewports is on, so legacy is unchanged and native still gets a flat blend. * redo * fix(next): avoid top-level dicomWebUtils destructure crash on boot getSopClassHandlerModule destructured transferDenaturalizedDataset / fixMultiValueKeys from dicomWebUtils at module-eval time. This module and @ohif/extension-default form a circular import, so dicomWebUtils can be undefined at eval time depending on bundler module order; the top-level destructure then throws (Cannot destructure property ... of dicomWebUtils as it is undefined) and crashes app boot before the Layout renders, which surfaced as the Playwright globalSetup warmup timing out on [data-cy=Layout]. Access the utils lazily at call time inside getDICOMwebMetadata instead. * fix(next): guard remount no-op path and defer legacy camera snapshot - CornerstoneViewportService.updateViewport: backend.remount() is typed Promise<void> | undefined and returns undefined for viewport families with no re-mount path; guard before .then() so those families no longer throw. - LegacyViewportBackend.remount: take the camera snapshot inside the volume branch (the only consumer) instead of before the family checks, so families without a camera surface no-op safely and the stack path skips a dead call. Addresses CodeRabbit review findings on PR #6101. * feat(next): add ?cpu=true URL opt-in to force the CPU render path Mirrors the ?useNextViewports opt-in: a cpu URL query param overrides appConfig.useCPURendering per-session (?cpu, =true, or =1 enable it). Wired through cornerstone.setUseCPURendering in init, whose global flag is consulted by the GenericViewport PlanarRenderPathDecisionService for both the image (CPU_IMAGE) and volume (CPU_VOLUME) paths, so under useNextViewports a single ?cpu=true forces the next viewport onto CPU. Extracted the shared query-param parsing into resolveBooleanUrlOptIn. * feat(cornerstone): select render backends via viewportRendering param Replaces the boolean cpu URL param with viewportRendering=cpu|webgl|auto (or any backend registered via cornerstone registerRenderBackend, e.g. a webgpu backend), mapped to the cornerstone render-backend registry. A per-viewport-type override (e.g. orthographic.viewportRendering=cpu) is passed as the per-mount renderBackend option on native planar mounts. The global value also drives the legacy useCPURendering flag so legacy viewports follow the same selection, letting a session force GPU when the deployed config defaults to CPU. * fix(dicom-seg): store overlapping segmentations as binary SEG A LABELMAP SEG frame stores a single label per voxel, so the labelmap encoder cannot represent overlapping segments (the later layer wins). When the export produces multiple overlapping layers and the resolved store mode is labelmap, switch that store to the binary SEG encoding, which writes overlapping segments as separate frames referencing the same source slice. * fix(cornerstone): apply review fixes to viewport backends - Anchor the cached-volume lookup in NextViewportAdapter to the loaderSchema:displaySetInstanceUID id shape instead of a substring match, so a derived volume id embedding the same UID cannot resolve. - Keep the viewing orientation on fitViewportToWindow (scaleBy 0) for native planar viewports, matching legacy resetCamera semantics. - Release legacy WSI metadata-provider registrations through the same ref-counted DataIdRegistry the native backend uses, so entries are removed on viewport disable and service destroy. * test(cornerstone): use real volumeId shape in adapter contract test The anchored cached-volume lookup rejects ids that merely embed the display set UID, so the mock now uses the real volumeLoaderSchema:displaySetInstanceUID shape and asserts an embedded UID id does not match. * docs(config): document useNextViewports and viewportRendering in default config Gives deployments an obvious place to opt into the native Generic Viewport path and configure its render backend. Both stay off/auto by default; the explicit false preserves the opt-in contract. * refactor(config): group next viewport settings under genericViewports Replaces the two top-level app config keys (useNextViewports, viewportRendering) with one genericViewports object: { enabled, viewportRendering }. The URL params are unchanged. * chore(deps): bump cornerstone packages to 5.4.15 Picks up the planar initial-slice remap fix (cornerstonejs/cornerstone3D#2799): opening a SEG display set now lands on the first segmented slice instead of its mirror when the cached volume reverses the imageId ordering. * test(segmentation): verify overlapping SEG rendering * fix(sr): make SCOORD3D hydration deterministic --------- Co-authored-by: Bill Wallace <wayfarer3130@gmail.com> * fix(pt): resolve Philips PET private SUV bulkdata before scaling (#6096) * fix(pt): resolve Philips PET private SUV bulkdata before scaling When a DICOMweb server delivers the Philips PET private tags SUVScaleFactor (7053,1000) / ActivityConcentrationScaleFactor (7053,1009) as bulkdata, dcmjs naturalization leaves them as { BulkDataURI } objects. These were fed verbatim to calculate-suv, which treats the object as a valid value and silently corrupts the SUV scaling factors. Resolve these scalar private tags to numbers during ingestion - in both the lazy (async) and non-lazy (sync) DICOMweb metadata paths, before INSTANCES_ADDED fires - by decoding the bulkdata (VR-aware: DS/IS text or little-endian FL/FD). Harden getPTImageIdInstanceMetadata to coerce values to finite numbers (reusing @ohif/core utils.toNumber) and reject unresolved bulkdata objects so they can never reach calculate-suv. Share the bulkdata-attach helper between both metadata paths. Adds unit tests for the bulkdata decoder/resolver and for getPTImageIdInstanceMetadata. * refactor(bulkdata): move PET bulkdata resolution to a generic core tag registry Generalize resolvePETPrivateScalarBulkData into a datasource-agnostic utils.resolveBulkDataTags in @ohif/core, backed by a static tag registry seeded with the Philips PET SUV/activity-concentration scalar tags and extensible via registerResolvedBulkDataTags. * fix(bulkdata): strip NUL padding and refresh qido auth before resolution Address review feedback: - decodeText now strips NUL (0x00) padding, which String.trim() leaves intact, so NUL-padded DS/IS values no longer decode as NaN. Adds a regression test. - refresh qidoDicomWebClient.headers before resolveBulkDataTags in both series-metadata paths; retrieveBulkData is bound to qidoDicomWebClient, matching every other qido op in this file. * fix(dicomweb): await deferred metadata storage * fix(dicomweb): support single-part bulkdata responses * chore(version): Update package versions to 3.13.0-beta.123 [skip ci] * ui(ContextMenuViewport): move to ui-next with theme support (#6008) * fix(ui): clean up remaining legacy ui usage (#6140) * Update button on DataSourceSelector * Removed dead code ToolbarButtonNestedMenu, button spacing * chore(version): Update package versions to 3.13.0-beta.124 [skip ci] * fix(cli): link and unlink extensions/modes with pnpm instead of yarn (#6144) * chore(version): Update package versions to 3.13.0-beta.125 [skip ci] * chore(deps): bump @cornerstonejs to 5.6.1 and @kitware/vtk.js to 36.4.1 (#6147) * chore(version): Update package versions to 3.13.0-beta.126 [skip ci] * fix(onboarding): use shepherd.js 15 to fix tour positioning crash (#6148) - Upgrade shepherd.js 13.0.3 -> 15.2.2 (uses floating-ui ^1.7.5, matching OHIF's own middleware) so the tour's detectOverflow crash is resolved; drop react-shepherd (its React 19 jsx-runtime is not recognized by React 18) and import Shepherd from shepherd.js directly. - Fold the MPR guidance into the Changing Layout step and remove the selectLayout step, which anchored to a dropdown item that is removed when the menu closes. * chore(version): Update package versions to 3.13.0-beta.127 [skip ci] * fix(dynamic-volume): restore kinetic analysis and segment stats on 4D volumes (#6149) Bumps @cornerstonejs to 5.6.2 (includes cornerstonejs/cornerstone3D#2808), resolves the chart mask via getOrCreateSegmentationVolume, and registers the chart display set with the hanging protocol. * chore(version): Update package versions to 3.13.0-beta.128 [skip ci] * fix(deps): pin websocket-driver to 0.7.5 (#6156) websocket-driver <0.7.5 reaches the tree only via platform/docs (@docusaurus/core > webpack-dev-server > sockjs > faye-websocket). Forcing 0.7.5 through the pnpm-workspace overrides block clears the high-risk audit gate in the BUILD_PACKAGES_QUICK CI job, which runs `pnpm audit --audit-level high` whenever pnpm-lock.yaml changes. GHSA-xv26-6w52-cph6. * chore(version): Update package versions to 3.13.0-beta.129 [skip ci] * fix(cornerstone): bump cornerstone3D to 5.6.4 for mobile volume rendering (#6155) Bumps all @cornerstonejs/* packages 5.6.2 -> 5.6.4 to pick up the fix rendering volume viewports without float-linear filtering, resolving volume rendering on mobile GPUs lacking OES_texture_float_linear for 32F textures (cornerstone3D #2031). * chore(version): Update package versions to 3.13.0-beta.130 [skip ci] * fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR (#6157) In the segmentation, usAnnotation and preclinical-4d modes the MPR Crosshairs tool was configured with no mouse binding and disableOnPassive: true. Without a binding it activates on plain Primary, so the moment another Primary tool in the same `mpr` group (brush, zoom, pan) is activated from the toolbar, the setToolActive handler evicts Crosshairs and — because of disableOnPassive — fully disables it, making them mutually exclusive. Measurement tools were unaffected only because they are not members of the `mpr` group. Bind Crosshairs to Primary+Shift (matching the longitudinal/tmtv modes) so it lives on its own binding and stays active alongside brush/zoom/pan. Fixes #6152 * chore(version): Update package versions to 3.13.0-beta.131 [skip ci] * fix(cornerstone): bump cornerstone3D to 5.6.7 (#6161) Bumps all @cornerstonejs/* packages 5.6.4 -> 5.6.7: SEG orientation string-IOP fix (#2816), invert-on-scroll fix (#2814), and websocket-driver 0.7.5 pin (5.6.5). * chore(version): Update package versions to 3.13.0-beta.132 [skip ci] * fix(worklist): disable supportsFuzzyMatching on staticWado AWS sources so patient-name search doesn't return empty (#6163) Co-authored-by: Alireza <ar.sedghi@gmail.com> * chore(version): Update package versions to 3.13.0-beta.133 [skip ci] * fix(cornerstone): bump cornerstone3D to 5.6.8 (#6165) * fix(cornerstone): bump cornerstone3D to 5.6.8 Bumps all @cornerstonejs/* packages 5.6.7 -> 5.6.8: contour segmentation undo/redo fix (cornerstonejs/cornerstone3D#2817) * fix(deps): pin adm-zip to 0.6.0 dc…
Context
add a console log after
const { PixelSpacing, type } = getPixelSpacingInformation(instance) || {};
in metadataprovider
see imageId in multiframe case is wrong even if you scroll always show the first one
try this case
https://viewer-dev.ohif.org/viewer?StudyInstanceUIDs=1.2.276.0.7230010.3.1.2.2723277605.10168.1676054414.178
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Greptile Summary
This PR fixes a bug where all frames of a multiframe DICOM image always reported the first frame's
imageId. The root cause is thatDicomMetadataStorekeeps only one instance per SOPInstanceUID;combineFrameInstancereturns a new per-frame object but that object inheritsimageIdfrom the parent, which always pointed to frame 1. The fix explicitly stampsresult.imageId = imageIdaftercombineFrameInstancereturns. It also removes thegetFrameInformationFromURLhelper (no external callers remain) and replaces URL-scraping with readinguids.frameNumberstored at ingestion time, while adding an explicit null-guard that was missing in the previous code.Confidence Score: 5/5
Safe to merge — the fix is narrowly scoped, no regressions found across all existing data source call sites
No P0/P1 issues found. The imageId stamp is correct: for multiframe images combineFrameInstance returns a new per-frame Object.create() object so only that frame-specific cached instance is mutated, never the shared DicomMetadataStore instance. The new uids.frameNumber fallback is equivalent to the old URL-scraping for every current data source. The removed getFrameInformationFromURL has zero callers remaining in the repo.
No files require special attention
Important Files Changed
Comments Outside Diff (1)
platform/core/src/services/DicomMetadataStore/DicomMetadataStore.ts, line 323 (link)window._model=_modelis leftover debug code that leaks the internal_modelobject (which holds the full DICOM studies/series/instances tree) onto the globalwindowobject. This bypasses encapsulation, is a TypeScript type error (no semicolon, implicitanyonwindow), and must not ship.Reviews (3): Last reviewed commit: "fix(MetadataProvider): Add check for und..." | Re-trigger Greptile
Summary by CodeRabbit
frame/frameNumberfor query-based URIs and preferring exact frame-specific matches.