Skip to content

revert: rename DisplaySet.frameOfReferenceUID back to FrameOfReferenceUID#5943

Merged
jbocce merged 2 commits into
OHIF:masterfrom
GhadeerAlbattarni:revert/rename-to-FrameOfReferenceUID
Apr 6, 2026
Merged

revert: rename DisplaySet.frameOfReferenceUID back to FrameOfReferenceUID#5943
jbocce merged 2 commits into
OHIF:masterfrom
GhadeerAlbattarni:revert/rename-to-FrameOfReferenceUID

Conversation

@GhadeerAlbattarni
Copy link
Copy Markdown
Collaborator

@GhadeerAlbattarni GhadeerAlbattarni commented Apr 6, 2026

Summary

  • Reverts the frameOfReferenceUID property on the DisplaySet type back to FrameOfReferenceUID

Context

A previous PR #5900 renamed FrameOfReferenceUIDframeOfReferenceUID on the DisplaySet type. This PR reverts that specific change.

Files Changed

  • platform/core/src/types/DisplaySet.ts
  • extensions/default/src/getSopClassHandlerModule.js
  • extensions/cornerstone/src/getSopClassHandlerModule.js
  • extensions/cornerstone/src/components/ViewportDataOverlaySettingMenu/utils.ts
  • extensions/cornerstone-dicom-seg/src/getSopClassHandlerModule.ts
  • extensions/cornerstone-dicom-rt/src/getSopClassHandlerModule.ts
  • extensions/cornerstone-dicom-sr/src/getSopClassHandlerModule.ts
  • extensions/cornerstone-dicom-sr/src/utils/hydrateStructuredReport.ts
  • extensions/dicom-microscopy/src/services/MicroscopyService.ts
  • extensions/dicom-microscopy/src/utils/getSourceDisplaySet.js

Testing

Checklist

PR

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

Code

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

Public Documentation Updates

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

Tested Environment

  • [] OS:
  • [] Node version:
  • [] Browser:

Greptile Summary

This PR reverts the rename introduced in PR #5900, restoring DisplaySet.FrameOfReferenceUID (PascalCase) across the type definition and all call sites. The change is consistent with the DICOM-standard casing used throughout the rest of the codebase.

All 10 explicitly listed files are updated correctly. However, one call site was missed:

  • extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts lines 1336\u20131337 \u2014 _getFrameOfReferenceUID still reads displaySet.frameOfReferenceUID (old lowercase name). The property will always be undefined, causing the function to skip the early-return and rely on the images[0].FrameOfReferenceUID fallback. For display sets without an images array the function silently returns undefined, breaking Frame-of-Reference resolution for those display sets.

Confidence Score: 4/5

Not safe to merge as-is — one missed call site in CornerstoneViewportService will silently return undefined for image-less display sets.

Score of 4 reflects a single real P1 defect: a missed property rename in CornerstoneViewportService that causes _getFrameOfReferenceUID to skip its primary return path and rely on a fallback that fails for image-less display sets.

extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts (lines 1336-1337)

Important Files Changed

Filename Overview
platform/core/src/types/DisplaySet.ts Type definition updated: frameOfReferenceUID → FrameOfReferenceUID. Straightforward and correct.
extensions/default/src/getSopClassHandlerModule.js Display-set construction updated to use FrameOfReferenceUID as the property key.
extensions/cornerstone/src/getSopClassHandlerModule.js Shorthand property renamed from frameOfReferenceUID: FrameOfReferenceUID to FrameOfReferenceUID.
extensions/cornerstone/src/components/ViewportDataOverlaySettingMenu/utils.ts Frame-of-reference comparison updated to FrameOfReferenceUID; separate segmentation.frameOfReferenceUID usage at line 162 is on a segmentation service object, not a DisplaySet, so it is unrelated.
extensions/cornerstone-dicom-seg/src/getSopClassHandlerModule.ts SEG display-set construction and deferred update both correctly switched to FrameOfReferenceUID.
extensions/cornerstone-dicom-rt/src/getSopClassHandlerModule.ts RTSTRUCT display-set construction and two assignment paths all updated to FrameOfReferenceUID.
extensions/cornerstone-dicom-sr/src/getSopClassHandlerModule.ts _measurementBelongsToDisplaySet predicate updated to compare against displaySet.FrameOfReferenceUID.
extensions/cornerstone-dicom-sr/src/utils/hydrateStructuredReport.ts Both getDisplaySetsBy filter callbacks updated to use FrameOfReferenceUID.
extensions/dicom-microscopy/src/services/MicroscopyService.ts Destructuring and filter conditions updated to FrameOfReferenceUID.
extensions/dicom-microscopy/src/utils/getSourceDisplaySet.js Display-set find predicate updated to FrameOfReferenceUID.

Comments Outside Diff (2)

  1. extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts, line 1336-1337 (link)

    P0 Missed rename causes silent undefined return

    _getFrameOfReferenceUID still accesses displaySet.frameOfReferenceUID (lowercase), but this PR renames that property to FrameOfReferenceUID on the DisplaySet type. JavaScript property access is case-sensitive, so displaySet.frameOfReferenceUID will always be undefined after this change. The function will silently fall through to the SEG/RTSTRUCT/images fallbacks — meaning regular display sets (e.g., those created by the default or cornerstone SOP class handlers that set FrameOfReferenceUID) will never have their frame of reference UID correctly returned by this method, potentially breaking viewport synchronization and any feature that relies on matching display sets by frame of reference.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
    Line: 1336-1337
    
    Comment:
    **Missed rename causes silent `undefined` return**
    
    `_getFrameOfReferenceUID` still accesses `displaySet.frameOfReferenceUID` (lowercase), but this PR renames that property to `FrameOfReferenceUID` on the `DisplaySet` type. JavaScript property access is case-sensitive, so `displaySet.frameOfReferenceUID` will always be `undefined` after this change. The function will silently fall through to the `SEG`/`RTSTRUCT`/`images` fallbacks — meaning regular display sets (e.g., those created by the default or cornerstone SOP class handlers that set `FrameOfReferenceUID`) will never have their frame of reference UID correctly returned by this method, potentially breaking viewport synchronization and any feature that relies on matching display sets by frame of reference.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts, line 1336-1337 (link)

    P1 Missed revert: still reads old frameOfReferenceUID property

    This function reads displaySet.frameOfReferenceUID (lowercase), but after this revert PR the canonical property is FrameOfReferenceUID (uppercase). The check on line 1336 will always evaluate to false/undefined for every display set, silently bypassing the intended early-return path and falling through to the images[0].FrameOfReferenceUID fallback.

    For display sets that have images, the fallback still returns the correct value, so the regression is subtle. However, for display sets that reach this function and have no images array (or an empty one), the function returns undefined instead of the correct Frame of Reference UID — breaking viewport registration for those cases.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
    Line: 1336-1337
    
    Comment:
    **Missed revert: still reads old `frameOfReferenceUID` property**
    
    This function reads `displaySet.frameOfReferenceUID` (lowercase), but after this revert PR the canonical property is `FrameOfReferenceUID` (uppercase). The check on line 1336 will always evaluate to `false`/`undefined` for every display set, silently bypassing the intended early-return path and falling through to the `images[0].FrameOfReferenceUID` fallback.
    
    For display sets that have `images`, the fallback still returns the correct value, so the regression is subtle. However, for display sets that reach this function and have no `images` array (or an empty one), the function returns `undefined` instead of the correct Frame of Reference UID — breaking viewport registration for those cases.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
Line: 1336-1337

Comment:
**Missed revert: still reads old `frameOfReferenceUID` property**

This function reads `displaySet.frameOfReferenceUID` (lowercase), but after this revert PR the canonical property is `FrameOfReferenceUID` (uppercase). The check on line 1336 will always evaluate to `false`/`undefined` for every display set, silently bypassing the intended early-return path and falling through to the `images[0].FrameOfReferenceUID` fallback.

For display sets that have `images`, the fallback still returns the correct value, so the regression is subtle. However, for display sets that reach this function and have no `images` array (or an empty one), the function returns `undefined` instead of the correct Frame of Reference UID — breaking viewport registration for those cases.

```suggestion
    if (displaySet.FrameOfReferenceUID) {
      return displaySet.FrameOfReferenceUID;
```

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

Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 6, 2026

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 66b0ffb
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/69d3c21cad737b0008e4413f
😎 Deploy Preview https://deploy-preview-5943--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@GhadeerAlbattarni GhadeerAlbattarni requested a review from jbocce April 6, 2026 14:18
@jbocce jbocce merged commit 0e933c2 into OHIF:master Apr 6, 2026
8 of 9 checks passed
wayfarer3130 added a commit that referenced this pull request Apr 15, 2026
* fix: Use newer ONNX version and load without errors

* Only changes to enable SAM again

* fix(seg hydration): auto-hydrate RT struct on second load with disableConfirmationPrompts (#5875)

* chore(version): Update package versions to 3.13.0-beta.34 [skip ci]

* fix(Threshold tool): Threshold tool no longer becomes deselected when the Dynamic option is selected (#5884)

fix(Threshold tool): Added 'ThresholdCircularBrushDynamic' to the toolNames array so the evaluator correctly recognizes it as an active state for the Threshold button when Dynamic mode is selected.

* chore(version): Update package versions to 3.13.0-beta.35 [skip ci]

* fix: Modalities in study list should select starts with as primary (#5886)

* chore(version): Update package versions to 3.13.0-beta.36 [skip ci]

* fix(security): Bump tar version to address CVE-2026-31802. (#5893)

* chore(version): Update package versions to 3.13.0-beta.37 [skip ci]

* fix(segmentation): Display "No description S:{series number} {modality}" for segmentations with no label. (#5874)

* Bump CS3D dependency to get the fallbackLabel field additions.

* chore(version): Update package versions to 3.13.0-beta.38 [skip ci]

* fix(window level): The window level value is not displayed by default on all the viewports when selecting common/custom layout and TMTV. (#5865)

* fix(window level): Set up listener for viewport availability such that the initial window level can be read and displayed.

* PR feedback.

* PR feedback.

---------

Co-authored-by: Bill Wallace <wayfarer3130@gmail.com>

* chore(version): Update package versions to 3.13.0-beta.39 [skip ci]

* fix(security): Bump flattened version to address CVE-2026-32141. (#5897)

* chore(version): Update package versions to 3.13.0-beta.40 [skip ci]

* fix(sr-hydration): enable hydration and arrow navigation for 3D SR measurements (#5887)

Joe is away, so approving based on the code having the requested change, and otherwise looking good/passing tests.

* fix(sr-hydration): enable hydration and arrows navigation for 3D SR measurements

* test: add automated test for SR measurement navigation with arrows after hydration

* add cross-study warning in the 3D branch

* test: address reviewer feedback for the test

* fix: support 3D and 2D annotations for SR hydration

* test: improve navigation to first image

---------

Co-authored-by: Bill Wallace <wayfarer3130@gmail.com>

* chore(version): Update package versions to 3.13.0-beta.41 [skip ci]

* feat: Add combined build (#5895)

* Add combined build

* Link script location update

* Security and validation fixes

* Allow specifying target path in PR description

* fix: Version match

* Fix build detection issue

* fix: Playwright deploy

* Separate out the branch merge guard

* Update docs and link info

* test: Update the layout change to wait for network idle

* Move audit late so the rest of the build can be worked on

* Add text with network check to ensure we see this change is updated

* Attempt to fix the mpr loading on ohif-downstream

* PR review comments

* Update docs

* Update to CS3D 4.20.0

* PR comments

* Add log on ohif-integration builds

* Update build test

* Removed unused space to kickoff build

* chore(version): Update package versions to 3.13.0-beta.42 [skip ci]

* fix(SR): Added support for spline and live wire SR items. (#5870)

* fix(SR): Added support for spline and live wire SR items.

* Apply suggestion from @greptile-apps[bot]

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* Add a script to checkout a worktree for test builds

* fix: Allow download for testing sr validator

* Remove script that wasn't intended to be included

* Bump CS3D version.

* PR comments - simplify code and use single codepath for download

* Allow both download and save buttons for SEG and RTSTRUCT

---------

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: Bill Wallace <wayfarer3130@gmail.com>

* chore(version): Update package versions to 3.13.0-beta.43 [skip ci]

* chore(tests): contour segment interactions e2e tests - rename and togglevisibility (#5891)

* chore(version): Update package versions to 3.13.0-beta.44 [skip ci]

* chore(refactor): use public appConfig getter instead of private _appConfig field (#5923)

* chore(version): Update package versions to 3.13.0-beta.45 [skip ci]

* refactor(tests): update viewport page object usage to async and update all effected tests (#5927)

* chore(version): Update package versions to 3.13.0-beta.46 [skip ci]

* fix: prevent black viewport when navigating series with client-created segmentation (#5919)

* chore(version): Update package versions to 3.13.0-beta.47 [skip ci]

* fix(measurement): Restore viewport interactivity when deleting in-progress Spline or Livewire measurement (#5905)

* chore(version): Update package versions to 3.13.0-beta.48 [skip ci]

* fix(segmentation): restrict overlay segmentation menu to same frame of reference as viewport background display set  (#5900)

- Add FrameOfReferenceUID to SEG and RTSTRUCT displaySet in SOP Class Handlers so the FOR is available for filtering
- Sync optimisticOverlayDisplaySets when background display set changes so the overlay menu reflects the correct state after a background switch
- Add FOR matching guard to the hydrate segmentation synchronizer to prevent the hydration synchronizer from blindly mirroring segmentations from a source viewport to a target viewport if their primary Frames of Reference do not align.
- fix segmentation overlay order reversal on viewport re-render

* chore(version): Update package versions to 3.13.0-beta.49 [skip ci]

* fix(security): update dependencies to fix security vulnerabilities (#5936)

* chore(version): Update package versions to 3.13.0-beta.50 [skip ci]

* fix(security): Update yarn.lock that was missed in PR #5936. (#5940)

* chore(version): Update package versions to 3.13.0-beta.51 [skip ci]

* feat(component): Adds SmartScrollbar to ui-next - OHIF-2558 (#5924)

Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com>

* fix(defaultRouteInit): pass sorted display sets to hanging protocol for deterministic viewport order (#5933)

fix: pass sorted display sets to hanging protocol for deterministic viewport order

The `applyHangingProtocol` function already sorts display sets by modality
priority and series number into `sortedDisplaySets`, but the unsorted
`displaySets` array was being passed to `hangingProtocolService.run()`.

This caused non-deterministic viewport ordering across page loads because
`displaySetService.getActiveDisplaySets()` returns display sets in creation
order, which depends on asynchronous network responses.

Made-with: Cursor

* chore(version): Update package versions to 3.13.0-beta.52 [skip ci]

* revert: rename DisplaySet.frameOfReferenceUID back to FrameOfReferenceUID (#5943)

* chore(version): Update package versions to 3.13.0-beta.53 [skip ci]

* fix(cornerstone): read FrameOfReferenceUID from display set in viewport service (#5950)

* chore(version): Update package versions to 3.13.0-beta.54 [skip ci]

* fix: ignore auth in git (#5955)

* chore(version): Update package versions to 3.13.0-beta.55 [skip ci]

* ONNX latest version

* chore(version): Update package versions to 3.13.0-beta.56 [skip ci]

* bun lock

* fix high sev mathjs issue

* Revert onnx changes

* Update to recent CS3D version

* Undo unneeded change

* Add null check

* Undo unneeded change

---------

Co-authored-by: Ghadeer Albattarni <165973963+GhadeerAlbattarni@users.noreply.github.com>
Co-authored-by: ohif-bot <danny.ri.brown+ohif-bot@gmail.com>
Co-authored-by: Joe Boccanfuso <109477394+jbocce@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: diattamo <mmddiatta@gmail.com>
Co-authored-by: Pedro Köhler <pedrokohlerbh@gmail.com>
Co-authored-by: Dan Rukas <dan.rukas@gmail.com>
Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com>
Co-authored-by: Alireza <ar.sedghi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants