Skip to content

fix(dicom-json): handle multiframe image ids#6121

Open
driavysinus wants to merge 1 commit into
OHIF:masterfrom
driavysinus:bugfix/dicomjson-multiframe
Open

fix(dicom-json): handle multiframe image ids#6121
driavysinus wants to merge 1 commit into
OHIF:masterfrom
driavysinus:bugfix/dicomjson-multiframe

Conversation

@driavysinus

@driavysinus driavysinus commented Jul 5, 2026

Copy link
Copy Markdown

Context

Fixes #5674.

DICOM JSON multiframe handling currently conflates two different JSON shapes:

  • a single DICOM instance with NumberOfFrames > 1
  • multiple DICOM JSON entries that point to individual frames of the same SOPInstanceUID

The issue sample uses the second shape: the series has NumberOfFrames: 2, but each item in series.instances already points at a specific frame URL. During ingestion, the series-level NumberOfFrames was copied onto each instance, so each per-frame entry could be treated as its own multiframe object. Later, getImageIdsForDisplaySet grouped instances by SOPInstanceUID and could reuse the first URL for non-multiframe per-frame entries.

Changes & Results

  • Added DICOM JSON-specific helper functions for instance metadata creation and display set image ID expansion.
  • Avoid copying a series-level NumberOfFrames onto every instance when the JSON already contains multiple frame entries.
  • Preserve NumberOfFrames for the single-instance multiframe case.
  • Keep the current per-frame URL for per-frame DICOM JSON entries that share a SOPInstanceUID.
  • Append &frame=N only when a single multiframe instance URL needs expansion and the URL does not already specify a frame.
  • Added unit coverage for the issue sample shape and the single-instance multiframe shape.

Testing

  • corepack pnpm install --frozen-lockfile
  • corepack pnpm exec prettier --check extensions/default/src/DicomJSONDataSource/index.js extensions/default/src/DicomJSONDataSource/index.test.js
  • git diff --check
  • corepack pnpm exec jest extensions/default/src/DicomJSONDataSource/index.test.js --runInBand --no-coverage

The targeted Jest suite passes with 6 tests. Manual viewer validation against a real reachable XA DICOM object was not performed; the issue attachment uses sample/redacted URLs that are not directly retrievable.

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: Windows 11
  • Node version: v24.14.0
  • Browser: Not run; datasource unit test only

Summary by CodeRabbit

  • New Features
    • Improved DICOM JSON image handling so multi-frame instances now generate the correct per-frame image IDs.
    • Added support for generating image IDs for display sets and individual instances more consistently.
    • Enhanced metadata mapping to better preserve and normalize instance details, including frame information.

@netlify

netlify Bot commented Jul 5, 2026

Copy link
Copy Markdown

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit a98026e
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/6a4a01318b00b30007e9aae0
😎 Deploy Preview https://deploy-preview-6121--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

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

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a6b5b434-2cd9-4b9c-bb5b-d281a579a3c6

📥 Commits

Reviewing files that changed from the base of the PR and between 2800f82 and a98026e.

📒 Files selected for processing (2)
  • extensions/default/src/DicomJSONDataSource/index.js
  • extensions/default/src/DicomJSONDataSource/index.test.js

📝 Walkthrough

Walkthrough

Refactors the DICOM JSON data source to introduce getDicomJSONImageId, getInstanceMetadata, and getDicomJSONImageIdsForDisplaySet helpers, replacing inline imageId/metadata construction logic across initialize, metadata, and display-set flows, addressing multiframe handling. Adds a new Jest test suite covering these helpers, and expands module exports.

Changes

DICOM JSON Multiframe ImageId Refactor

Layer / File(s) Summary
Core imageId and metadata helpers
extensions/default/src/DicomJSONDataSource/index.js
Adds getDicomJSONImageId to append &frame= for multi-frame instances when not already present, and getInstanceMetadata to merge instance/series/study metadata, backfill NumberOfFrames, and strip instances/series fields.
Display set imageId expansion
extensions/default/src/DicomJSONDataSource/index.js
Adds getDicomJSONImageIdsForDisplaySet, computing per-instance imageId arrays and expanding multi-frame instances via SOPInstanceUID.
Wiring helpers into data source flows
extensions/default/src/DicomJSONDataSource/index.js
Updates initialize, series.metadata, getImageIdsForDisplaySet, and getImageIdsForInstance to delegate to the new helpers, and expands exports to include the new functions.
Unit tests for new helpers
extensions/default/src/DicomJSONDataSource/index.test.js
Adds a Jest suite mocking @ohif/core and validating NumberOfFrames handling, frame query appending, wadoUriRoot fallback behavior, repeated SOPInstanceUID handling, and multiframe display-set expansion.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Viewer
  participant DicomJSONDataSource
  participant getDicomJSONImageIdsForDisplaySet
  participant getDicomJSONImageId
  participant MetadataProvider

  Viewer->>DicomJSONDataSource: getImageIdsForDisplaySet(displaySet)
  DicomJSONDataSource->>getDicomJSONImageIdsForDisplaySet: compute imageIds
  getDicomJSONImageIdsForDisplaySet->>getDicomJSONImageId: derive imageId per instance/frame
  getDicomJSONImageId-->>getDicomJSONImageIdsForDisplaySet: imageId with &frame= appended
  getDicomJSONImageIdsForDisplaySet-->>DicomJSONDataSource: imageId array
  DicomJSONDataSource->>MetadataProvider: addImageIdToUIDs(imageId)
  DicomJSONDataSource-->>Viewer: imageIds for rendering
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, semantic-release styled, and matches the multiframe DICOM JSON fix.
Description check ✅ Passed The PR description includes context, change summary, testing, and completed checklist items.
Linked Issues check ✅ Passed The changes address the multiframe DICOMJSON image-id and metadata handling required by #5674.
Out of Scope Changes check ✅ Passed The diff stays focused on DICOMJSON datasource helpers and tests, with no unrelated feature work.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@driavysinus
driavysinus marked this pull request as ready for review July 5, 2026 07:10
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.

[Bug] Multiframe DICOM do not work with DICOMJSON datasource

1 participant