fix(cornerstone-dicom-sr): handle content-less imaging measurement reports#6139
Conversation
…ports An Imaging Measurement Report can be stored without its report body: the ConceptNameCodeSequence identifies it as one, but ContentSequence (0040,A730) is absent. Such a report was classified on the concept name alone, so the loader called _getReferencedImagesList and _getMeasurements on the missing content. _getMeasurements calls .find on it and throws, taking the whole viewer down with "Something went wrong" — the study could not be opened at all. Classify a report that has no ContentSequence as a plain SR. Both the loader and the SR viewport already branch on isImagingMeasurementReport, so this keeps them off the measurement path that assumes content exists. The report opens empty in the SR text viewport, which now warns that it has no readable content. Reports that do have a ContentSequence are unaffected.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@wayfarer3130 @jbocce could one of you take a look? This is a small classification fix in the SR SOP class handler: an Imaging Measurement Report stored without its ContentSequence currently crashes the viewer outright ("Something went wrong"), because the measurement path calls |
GitHub Actions
Inactive
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe SR handler now identifies Imaging Measurement Reports without ChangesContentless SR handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 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 |
1 participant
Context
An Imaging Measurement Report can be stored without its report body — the
ConceptNameCodeSequenceidentifies it as an Imaging Measurement Report, butContentSequence(0040,A730) is absent. We hit this with real reports produced by a third-party system.OHIF classifies such a report as an Imaging Measurement Report on the concept name alone, then takes the measurement path on content that isn't there:
_load→_getReferencedImagesList(ContentSequence)and_getMeasurements(ContentSequence)withContentSequence === undefined._getMeasurementsimmediately calls.findon it, which throws.The exception escapes and takes down the viewer with the generic "Something went wrong" screen — the user can't open the study at all, not just that one report.
Changes & Results
extensions/cornerstone-dicom-sr/src/getSopClassHandlerModule.ts:conceptIsImagingMeasurementReport) from the resulting classification. A report whose concept says Imaging Measurement Report but which carries noContentSequenceis now flaggedisContentlessImagingMeasurementReportand classified as a plain SR (isImagingMeasurementReport: false)._load, which gates the measurement parsing onisImagingMeasurementReport) and the viewport (OHIFCornerstoneSRViewport, which routes on the same flag) already branch on that flag, downgrading it is enough to keep both off the measurement path that assumes content exists. No changes to the parsing code itself.extensions/cornerstone-dicom-sr/src/components/OHIFCornerstoneSRTextViewport.tsx:Before: opening a study containing a content-less Imaging Measurement Report crashes the viewer with "Something went wrong".
After: the report opens as an empty plain SR with a "This report has no readable content and cannot be displayed." warning; the rest of the study is unaffected. Healthy SRs — any report that has a
ContentSequence— are untouched and still take the full measurement path.Testing
To reproduce, you need an SR whose
ConceptNameCodeSequence.CodeValueis126000(Imaging Measurement Report) and which has noContentSequence(0040,A730).Regression check — a healthy Imaging Measurement Report (one with a
ContentSequence) still loads its measurements and renders in the measurement viewport, unchanged.This fix has been running in a downstream production deployment against the real reports that triggered the crash.
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
This fix is contributed by the University of Calgary, who are donating it back to OHIF. It was co-authored with Ron Leisti.
Summary by CodeRabbit