Repository navigation
fix(ai): keep auto segment mode from crashing on a viewport without a segmentation - #2953
Conversation
… segmentation LabelmapSlicePropagationTool re-points its ONNXSegmentationController at whichever viewport becomes active. With auto segment mode on, the next render of that viewport read segmentationId off the active segmentation, which is undefined when the viewport has none, and threw from the IMAGE_RENDERED listener. With that fixed, a second failure surfaced on the same switch: initViewport kept the propagation points gathered on the previous viewport, so once the new image was encoded the decoder ran them against a viewport whose labelmap tool had no preview to create, and createLabelmap threw on the undefined preview. Skip propagation where there is no active segmentation, and drop the previous viewport's points when the controller moves to another one.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe controller now clears leftover propagation points when it initializes a viewport. In auto-segment mode, it returns from the render listener when the active viewport has no active segmentation. Tests cover both behaviors. ChangesViewport safeguards
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change prevents stale propagation points from crossing viewports and avoids auto-segmentation errors when no segmentation exists. No actionable merge-blocking risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change reduces stale segmentation reuse and avoids rendering failures without expanding access or authority. Risk remains low because safe completion of predictions already running during viewport switches is not fully established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 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 |
Context
LabelmapSlicePropagationToolre-points itsONNXSegmentationControllerat whichever viewport becomes active: an application callssetToolConfigurationwith a newsourceViewportId,onSetToolConfigurationruns_init, and_initcallssegmentAI.initViewport(viewport). In a multi-viewport layout the newly active viewport often has no segmentation of its own. Two separate failures follow from that switch.1. The render listener throws. With auto segment mode on, the
IMAGE_RENDEREDlistener reads the active segmentation of the viewport and takessegmentationIdfrom it without a check:It throws on every render of that viewport while Labelmap Assist is on.
2. The decoder previews into a viewport that has no preview. This one was hidden behind the first.
initViewportresetscurrentImagebut keepsrandomPoints, the world points gathered from the previous viewport's segmentation.initViewportthen callstryLoad(), and when the encode of the new image finishes,tryLoadrebuildsthis.pointsfrom those stale points and runs the decoder.createLabelmapcallsthis.tool.addPreview(viewport.element)on the new viewport, which has no segmentation to preview into, getsundefinedback, and throws:This one needs a real prediction on the first viewport (paint a slice, scroll so points are collected), then a click on another viewport. On a viewport that does have its own segmentation, the same stale points would produce a preview seeded from the other viewport.
Changes & Results
viewportRenderedListener: when the viewport has no active segmentation, return before the propagation step. There is nothing to propagate from. The existing non-acquisition-plane branch right above it already returns the same way.initViewport: clearrandomPointstogether withcurrentImage, so points gathered on one viewport are never decoded against another.Before: with Labelmap Assist on, clicking a second viewport that has no segmentation throws from the render listener. After a prediction on the first viewport, it also throws from the decoder.
After: the switch is silent. Assist keeps predicting on the first viewport. Once the second viewport gets a segmentation of its own, Assist predicts there too, because the early return leaves
currentImageuntouched and the next render goes through normally.Testing
New
packages/ai/src/ONNXSegmentationController.test.ts:initViewportdoes not carry the previous viewport's propagation points over.pnpm jestinpackages/ai: 2 suites / 5 tests pass. Negative check: with the source change reverted, both new tests fail (TypeErrorfor the first).Manual, in an OHIF-based viewer with a multi-viewport cine MR layout (
@cornerstonejs/ai5.10.9 with this change applied to its dist):Checklist
PR
Code
Public Documentation Updates
Tested Environment
Summary by CodeRabbit