Skip to content

fix(dicom-json): correct frame indexing and clarify metadata requirements#6120

Open
driavysinus wants to merge 2 commits into
OHIF:masterfrom
driavysinus:docs/dicom-json-metadata-guidance
Open

fix(dicom-json): correct frame indexing and clarify metadata requirements#6120
driavysinus wants to merge 2 commits into
OHIF:masterfrom
driavysinus:docs/dicom-json-metadata-guidance

Conversation

@driavysinus

@driavysinus driavysinus commented Jul 5, 2026

Copy link
Copy Markdown

Context

Related to #3093.

The required metadata FAQ already documents the broad set of fields OHIF needs, but DICOM JSON users still have to decide which metadata to keep when reducing JSON payload size. This update clarifies the practical baseline metadata and modality-specific fields needed for display set creation, rendering, sorting, cine, ultrasound, PT SUV correction, SEG, RTSTRUCT, SR, PDF, and video handling.

Changes & Results

  • Fixed the DICOM JSON generator to emit zero-based frame query parameters for multi-frame objects.
  • Reworked the required metadata FAQ into baseline image metadata, rendering metadata, modality/feature metadata, and DICOM JSON specific notes.
  • Clarified PlanarConfiguration for color/RGB images, NumberOfFrames for multi-frame objects, and zero-based frame query parameters.
  • Expanded PT SUV correction notes to include the required radiopharmaceutical sequence fields used by OHIF's PT metadata helper.
  • Added a short DICOM JSON guide note about metadata pruning and multi-frame frame URLs.
  • Updated both current docs and the version-3.11 docs copy.

Testing

  • git diff --check
  • corepack pnpm dlx prettier@3.6.2 --check platform/docs/docs/faq/technical.md platform/docs/docs/configuration/dataSources/dicom-json.md platform/docs/versioned_docs/version-3.11/faq/technical.md platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md
  • Verified the current and versioned FAQ copies match after the update.

Full docs build was not run because the monorepo dependencies were not installed locally.

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; docs-only change

Summary by CodeRabbit

  • Documentation

    • Clarified that when using DICOM JSON (especially with metadata stripped for smaller payloads), required tags for display set creation, rendering, sorting, and modality-specific tools must be preserved.
    • Expanded multi-frame guidance, including conditional NumberOfFrames inclusion and zero-based frame URL indexing.
    • Reworked technical FAQ into clearer required vs. optional metadata tables and DICOM JSON-specific notes (including SR CodeSequence handling for measurements).
  • Bug Fixes

    • Corrected multi-frame generation so per-frame URLs use the expected zero-based frame numbering.

@netlify

netlify Bot commented Jul 5, 2026

Copy link
Copy Markdown

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit e8e358b
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/6a4d396f98f6da0008218312
😎 Deploy Preview https://deploy-preview-6120--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

📝 Walkthrough

Walkthrough

DICOM JSON documentation now describes required metadata, rendering and modality-specific tags, and DICOM JSON-specific rules for frame indexing and URL placement. The generator script now emits zero-based frame URLs for multi-frame instances.

Changes

DICOM JSON generator and frame URL behavior

Layer / File(s) Summary
Generator frame indexing and CLI output
.scripts/dicom-json-generator.js
Updates CLI usage output, callback formatting, per-series instance selection, and multi-frame URL generation to use zero-based frame values.

DICOM JSON metadata guidance docs

Layer / File(s) Summary
DICOM JSON metadata requirements docs
platform/docs/docs/configuration/dataSources/dicom-json.md, platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md, platform/docs/docs/faq/technical.md, platform/docs/versioned_docs/version-3.11/faq/technical.md
Replaces generic mandatory/optional metadata lists with structured guidance covering required tags, rendering metadata, modality-specific metadata, DICOM JSON-specific notes, and optional metadata descriptions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: documentation

Suggested reviewers: None identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise, semantic-release formatted, and accurately reflects the main changes to frame indexing and DICOM JSON metadata docs.
Description check ✅ Passed The description follows the template with Context, Changes & Results, Testing, and a completed Checklist.
✨ 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 06:48

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/docs/docs/configuration/dataSources/dicom-json.md`:
- Around line 45-46: The DICOM JSON docs wording is misleading because the
script does not create SR metadata; it only preserves existing SR-related fields
when they are already present in the source metadata. Update the copy in
dicom-json.md around the metadata guidance to say the script retains or carries
over CodeSequences for SR when available, rather than saying it will add them,
and keep the explanation aligned with the generator behavior.

In
`@platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md`:
- Around line 48-52: The DICOM JSON generator is emitting one-based frame URLs
while the docs specify that frame indexing must be zero-based. Update the logic
in the DICOM JSON generation path, especially the code that builds the
multi-frame URL in dicom-json-generator.js, so it uses frame values starting at
0 rather than 1. Keep the URL construction consistent wherever the frame query
parameter is generated, and verify any related frame loops or counters are
adjusted to match.
- Around line 45-46: The DICOM JSON documentation currently implies the script
generates SR metadata, but it should state that the script preserves and
forwards existing SR sequences from the source metadata instead. Update the
wording around the script description and the SR example to make it clear that
sequences such as ConceptNameCodeSequence are retained for display, not created
by the script.
🪄 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: 93a3c449-0635-4da1-9aaa-6d3febe1877d

📥 Commits

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

📒 Files selected for processing (4)
  • platform/docs/docs/configuration/dataSources/dicom-json.md
  • platform/docs/docs/faq/technical.md
  • platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md
  • platform/docs/versioned_docs/version-3.11/faq/technical.md

Comment thread platform/docs/docs/configuration/dataSources/dicom-json.md Outdated
Comment thread platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md Outdated
@driavysinus driavysinus changed the title docs(dicom-json): clarify metadata requirements fix(dicom-json): correct frame indexing and clarify metadata requirements Jul 18, 2026
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.

1 participant