docs(dicom-json): clarify metadata requirements#6120
Conversation
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughDocumentation-only update revising DICOM JSON data source pages and FAQ technical pages (both current and versioned-3.11 copies) to replace generic mandatory/optional metadata lists with structured tables covering baseline, rendering, and modality-specific metadata, plus new notes on multi-frame handling, UID consistency, and frame indexing. ChangesDICOM JSON Metadata Requirements Documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Related issues: None referenced. Related PRs: None referenced. Suggested labels: documentation Suggested reviewers: None identified. 🐰 PoemA rabbit reads the DICOM scroll, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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
📒 Files selected for processing (4)
platform/docs/docs/configuration/dataSources/dicom-json.mdplatform/docs/docs/faq/technical.mdplatform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.mdplatform/docs/versioned_docs/version-3.11/faq/technical.md
| Some modalities require additional metadata to be added to the JSON file. You can read more about the minimum and recommended metadata required for the viewer to work [here](../../faq/technical#what-are-the-list-of-required-metadata-for-the-ohif-viewer-to-work). We will handle this in the script. For example, the script will add the CodeSequences for SR in order to display the measurements in the viewer. | ||
| ::: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant documentation section and generator implementation.
git ls-files 'platform/docs/docs/configuration/dataSources/dicom-json.md' '.scripts/dicom-json-generator.js'
echo
echo '--- doc excerpt ---'
sed -n '35,60p' platform/docs/docs/configuration/dataSources/dicom-json.md
echo
echo '--- generator excerpt ---'
sed -n '160,300p' .scripts/dicom-json-generator.jsRepository: OHIF/Viewers
Length of output: 6312
Clarify that SR fields are retained, not synthesized platform/docs/docs/configuration/dataSources/dicom-json.md:45-46 — the generator only copies SR-related sequences when they already exist in the source metadata. Reword this to say the script preserves those fields when present, rather than “adding” them.
🧰 Tools
🪛 LanguageTool
[style] ~45-~45: Consider a more concise word here.
Context: ...cript will add the CodeSequences for SR in order to display the measurements in the viewer....
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.22.1)
[warning] 45-45: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 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/docs/docs/configuration/dataSources/dicom-json.md` around lines 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.
| Some modalities require additional metadata to be added to the JSON file. You can read more about the minimum and recommended metadata required for the viewer to work [here](../../faq/technical#what-are-the-list-of-required-metadata-for-the-ohif-viewer-to-work). We will handle this in the script. For example, the script will add the CodeSequences for SR in order to display the measurements in the viewer. | ||
| ::: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced files and inspect the relevant sections.
git ls-files 'platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md' \
'.scripts/dicom-json-generator.js'
echo '--- DOC EXCERPT ---'
sed -n '35,60p' platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md
echo '--- GENERATOR OUTLINE ---'
ast-grep outline .scripts/dicom-json-generator.js --view expanded || true
echo '--- GENERATOR RELEVANT SECTION ---'
sed -n '150,310p' .scripts/dicom-json-generator.jsRepository: OHIF/Viewers
Length of output: 7611
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- SEARCH FOR CODESEQUENCE REFERENCES ---'
rg -n "CodeSequence|CodeSequences|SR" .scripts/dicom-json-generator.js platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md
echo '--- PROCESS INSTANCE SECTION ---'
sed -n '90,170p' .scripts/dicom-json-generator.jsRepository: OHIF/Viewers
Length of output: 3225
Clarify SR metadata is preserved, not generated
This script only forwards existing SR sequences (for example, ConceptNameCodeSequence) from the source metadata; it does not add them. Reword this as a retention requirement.
🧰 Tools
🪛 LanguageTool
[style] ~45-~45: Consider a more concise word here.
Context: ...cript will add the CodeSequences for SR in order to display the measurements in the viewer....
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.22.1)
[warning] 45-45: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 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/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md`
around lines 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.
| :::note | ||
| For multi-frame instances, include `NumberOfFrames` only when it matches the | ||
| source object and the image URLs you provide. If a URL points to a specific | ||
| frame, the `frame` query parameter is zero-based, so `frame=0` is the first | ||
| frame. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the documented note and the generator around the referenced lines.
printf '\n## docs snippet\n'
sed -n '40,60p' platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md
printf '\n## generator snippet\n'
sed -n '235,270p' .scripts/dicom-json-generator.js
printf '\n## search for frame usage\n'
rg -n "frame=|NumberOfFrames|zero-based|one-based|frame parameter" platform/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md .scripts/dicom-json-generator.jsRepository: OHIF/Viewers
Length of output: 3654
Make the frame index zero-based in the generator .scripts/dicom-json-generator.js:265 emits ?frame=${i} starting at 1, but the docs say frame=0 is the first frame. This shifts every multi-frame URL by one.
🤖 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/docs/versioned_docs/version-3.11/configuration/dataSources/dicom-json.md`
around lines 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.
1 participant
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
PlanarConfigurationfor color/RGB images,NumberOfFramesfor multi-frame objects, and zero-basedframequery parameters.version-3.11docs copy.Testing
git diff --checkcorepack 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.mdFull docs build was not run because the monorepo dependencies were not installed locally.
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Summary by CodeRabbit