feat: Add ability to use new display set split rules - #6137
Open
wayfarer3130 wants to merge 9 commits into
Open
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
wayfarer3130
had a problem deploying
to
fork-pr-approval
July 12, 2026 01:19 — with 
GitHub Actions
Failure
GitHub Actions
Failure
|
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:
📝 WalkthroughWalkthroughAdds opt-in metadata-driven display-set splitting with safe ChangesMetadata display-set splitting
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant DisplaySetService
participant CustomizationService
participant SplitRulesEngine
participant DisplaySetFactory
participant displaySetStore
DisplaySetService->>CustomizationService: read useMetadataDisplaySet
DisplaySetService->>SplitRulesEngine: group instances by splitRules
SplitRulesEngine-->>DisplaySetService: matched groups and unmatched instances
DisplaySetService->>DisplaySetFactory: createDisplaySetFromGroup
DisplaySetFactory->>displaySetStore: store split display set
DisplaySetService->>DisplaySetService: route unmatched instances to SOP handlers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
extensions/default/src/getSopClassHandlerModule.js (1)
15-15: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the unused second argument from
makeDisplaySetcalls
makeDisplaySetonly forwardsinstancesandappContexttomakeImageSetDisplaySet, soinstanceIndex/displaySets.lengthare dead arguments here. Remove them from the three call sites to avoid confusion.🤖 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 `@extensions/default/src/getSopClassHandlerModule.js` at line 15, Update the three call sites of makeDisplaySet to pass only the required instances argument, removing the unused instanceIndex and displaySets.length arguments while preserving the existing makeDisplaySet implementation.platform/core/src/types/DisplaySet.ts (1)
93-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a type annotation for the
displaySetServiceparameter.The
displaySetServiceparameter onupdateInstanceshas no type annotation, making it implicitlyany. ImportingDisplaySetServicedirectly would create a circular dependency (the services layer imports fromtypes/), so a minimal interface would preserve type safety without the architectural concern.♻️ Suggested interface to avoid circular dependency
export type DisplaySet = { displaySetInstanceUID: string; instances: InstanceMetadata[]; isReconstructable?: boolean; StudyInstanceUID: string; SeriesInstanceUID?: string; SeriesNumber?: number; SeriesDescription?: string; numImages?: number; unsupported?: boolean; Modality?: string; imageIds?: string[]; images?: unknown[]; label?: string; /** Flag indicating if this is an overlay display set (e.g., SEG, RTSTRUCT) */ isOverlayDisplaySet?: boolean; /** Flag indicating this is a derived dataset */ isDerived?: boolean; /** flag indicating if it supports window level */ supportsWindowLevel?: boolean; // Details about how to display: /** * A URL that can be used to display the thumbnail. Typically a data url * This can be set to null to avoid trying to display a thumbnail, eg for * display sets without a thumbnail. */ thumbnailSrc?: string; /** A fetch method to get the thumbnail */ getThumbnailSrc?(imageId?: string): Promise<string>; /** An opaque type of this viewport, used internally to specify which viewport to use */ viewportType; /** * A fetch URL to display the content. This is used for content such as * pdf display. */ renderedUrl?: string; /** * The instance UID of the display set that this display set references. * This is used to determine if the display set is a referenced display set. * It usually is for SEG, RTSTRUCT, etc. */ referencedDisplaySetInstanceUID?: string; /** * The FrameOfReferenceUID shared by every frame within this display set. * It will be undefined if the frames do not all share the same Frame of Reference. */ FrameOfReferenceUID?: string; SeriesDate?: string; SeriesTime?: string; instance?: InstanceMetadata; /** * The predecessor image id refers to the SOP instance that is currently loaded * into this display set for SEG/SR/RTSTRUCT type values. The name is chosen * for consistency when this value is used as the origin instance * for saving a new instance intended to replace this instance where the * new instance has a "predecessor sequence". */ predecessorImageId?: string; /** * isLoaded is used for display sets containing a load operation that * is required before the display set can be shown. This is separate from * isHydrated, which means it is loaded into view. */ isLoaded?: boolean; isHydrated?: boolean; isRehydratable?: boolean; /** * The name of the comparison function (for sort) to use when comparing display * sets that are coming from same series instanceUID. */ compareSameSeries?: string; + /** + * Minimal interface for the DisplaySetService methods that + * `updateInstances` needs, avoiding a circular import from + * `types/` into the services layer. + */ /** * The deterministic, rule-namespaced group key assigned by the * `@cornerstonejs/metadata` split-rules engine when this display set was * created via the `useMetadataDisplaySet` customization. Used to reconcile * re-splits of the same series with already-created display sets. */ splitKey?: string; /** The id of the split rule that created this display set, when applicable. */ splitRuleId?: string; /** * Incremental-merge hook for split-rule display sets. Intentionally named * differently from `addInstances` (the SOP-class-handler merge hook) so the * legacy handler loop never feeds unmatched instances into split-rule * display sets. Returns the updated display set, or undefined when the * display set cannot merge the instances. */ - updateInstances?(instances: InstanceMetadata[], displaySetService): DisplaySet | undefined; + updateInstances?( + instances: InstanceMetadata[], + displaySetService: DisplaySetServiceLike + ): DisplaySet | undefined; }; + +/** + * Minimal interface for the DisplaySetService methods that `updateInstances` + * callers need, avoiding a circular import from `types/` into the services layer. + */ +export interface DisplaySetServiceLike { + setDisplaySetMetadataInvalidated(displaySetInstanceUID: string): void; + getDisplaySetsForSeries(seriesInstanceUID: string): DisplaySet[]; +}🤖 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/types/DisplaySet.ts` around lines 93 - 112, Update the DisplaySet.updateInstances signature to replace the implicit-any displaySetService parameter with a minimal local interface describing the service members this hook uses. Define or reuse that interface within the types layer rather than importing DisplaySetService, preserving type safety without introducing a circular dependency.
🤖 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 `@extensions/default/src/displaySetSplitting/makeImageSetDisplaySet.ts`:
- Around line 36-42: Update the volumeLoaderUtility lookup in
makeImageSetDisplaySet to check whether getModuleEntry returns undefined before
accessing exports. If the utility is unavailable, throw a clear descriptive
error; otherwise preserve the existing getDynamicVolumeInfo extraction and
invocation.
In
`@platform/core/src/services/CustomizationService/expression/expression.test.ts`:
- Around line 137-149: Rename the test around compileExpression to describe
graceful null property access in templates rather than runtime errors, warnings,
or an undefined result. Keep the existing `${a.b.c}` assertion and setup
unchanged.
---
Nitpick comments:
In `@extensions/default/src/getSopClassHandlerModule.js`:
- Line 15: Update the three call sites of makeDisplaySet to pass only the
required instances argument, removing the unused instanceIndex and
displaySets.length arguments while preserving the existing makeDisplaySet
implementation.
In `@platform/core/src/types/DisplaySet.ts`:
- Around line 93-112: Update the DisplaySet.updateInstances signature to replace
the implicit-any displaySetService parameter with a minimal local interface
describing the service members this hook uses. Define or reuse that interface
within the types layer rather than importing DisplaySetService, preserving type
safety without introducing a circular dependency.
🪄 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: f0e9f3f4-e8d2-47ba-94e0-8102288831c9
📒 Files selected for processing (25)
extensions/default/package.jsonextensions/default/src/customizations/metadataDisplaySetCustomization.tsextensions/default/src/displaySetSplitting/makeDisplaySetFromInstanceGroup.tsextensions/default/src/displaySetSplitting/makeImageSetDisplaySet.tsextensions/default/src/displaySetSplitting/ohifDefaultSplitRules.test.tsextensions/default/src/displaySetSplitting/ohifDefaultSplitRules.tsextensions/default/src/getCustomizationModule.tsxextensions/default/src/getSopClassHandlerModule.jsplatform/app/public/customizations/index.htmlplatform/app/public/customizations/split/enableNewSplit.jsoncplatform/app/public/customizations/split/scoutSeries.jsoncplatform/core/src/services/CustomizationService/CustomizationService.function.test.tsplatform/core/src/services/CustomizationService/CustomizationService.tsplatform/core/src/services/CustomizationService/expression/compiler.tsplatform/core/src/services/CustomizationService/expression/expression.test.tsplatform/core/src/services/CustomizationService/expression/index.tsplatform/core/src/services/CustomizationService/expression/parser.tsplatform/core/src/services/CustomizationService/expression/tokenizer.tsplatform/core/src/services/DisplaySetService/DisplaySetService.test.tsplatform/core/src/services/DisplaySetService/DisplaySetService.tsplatform/core/src/services/DisplaySetService/displaySetStore.test.tsplatform/core/src/services/DisplaySetService/displaySetStore.tsplatform/core/src/services/DisplaySetService/normalizeSplitRules.tsplatform/core/src/types/DisplaySet.tsplatform/docs/docs/platform/services/customization-service/displaySetSplitting.md
wayfarer3130
had a problem deploying
to
fork-pr-approval
July 14, 2026 15:33 — with
GitHub Actions
Failure
GitHub Actions
Failure
…use-metadata-display-set
wayfarer3130
had a problem deploying
to
fork-pr-approval
July 15, 2026 13:47 — with
GitHub Actions
Failure
GitHub Actions
Failure
wayfarer3130
had a problem deploying
to
fork-pr-approval
July 15, 2026 14:10 — with
GitHub Actions
Failure
GitHub Actions
Failure
wayfarer3130
temporarily deployed
to
fork-pr-approval
July 15, 2026 15:06 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Viewers
 |
||||||||||||||||||||||||||||
| Project |
Viewers
|
| Branch Review |
feat/customization-use-metadata-display-set
|
| Run status |
|
| Run duration | 01m 46s |
| Commit |
|
| Committer | Bill Wallace |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
28
|
| View all changes introduced in this branch ↗︎ | |
wayfarer3130
had a problem deploying
to
fork-pr-approval
July 27, 2026 14:43 — with
GitHub Actions
Failure
GitHub Actions
Failure
…use-metadata-display-set
wayfarer3130
had a problem deploying
to
fork-pr-approval
July 28, 2026 19:43 — with
GitHub Actions
Failure
GitHub Actions
Failure
wayfarer3130
temporarily deployed
to
fork-pr-approval
July 28, 2026 21:23 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This PR starts using the @cornerstonejs/metadata display set storage, as well as adding the ability to use the new display set split rules via enabling them through a customization. An example of a split rule that splits the first image of CT's as a "SCOUT" is created to demo how to use the new customization design.
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Summary by CodeRabbit
$functionsupport for declarative customization logic (with graceful failure on invalid expressions).$functionusage.