feat: update to ohif 3.11 #1855
Conversation
Signed-off-by: yanq <[email protected]>
for more information, see https://pre-commit.ci Signed-off-by: yanq <[email protected]>
Signed-off-by: Qingyu Yan <[email protected]> Signed-off-by: yanq <[email protected]>
WalkthroughThe pull request upgrades the MONAI Label extension and mode to OHIF 3.11.0, updates Cornerstone dependencies to 3.32.5, migrates UI components to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 5
🧹 Nitpick comments (7)
plugins/ohifv3/modes/monai-label/babel.config.js (1)
13-13: Inconsistent quote style across environments.Line 13 uses single quotes (
'@babel/preset-typescript') while lines 29 and 38 use double quotes ("@babel/preset-typescript") for the same preset. Standardize the quote style across all three environments for consistency.🔎 Proposed fix to standardize quotes
presets: [ [ // TODO: https://babeljs.io/blog/2019/03/19/7.4.0#migration-from-core-js-2 '@babel/preset-env', { modules: 'commonjs', debug: false, }, - '@babel/preset-typescript', + "@babel/preset-typescript", ],Alternatively, if your project uses a specific quote style convention (e.g., single quotes throughout), update lines 29 and 38 to match line 13 instead.
Also applies to: 29-29, 38-38
plugins/ohifv3/extensions/monai-label/src/components/OptionsInputDialog.tsx (2)
18-18: Consider adding TypeScript interface for component props.This is a
.tsxfile but the component props are untyped. Adding an interface improves type safety and documentation.🔎 Suggested type definition
+interface OptionsInputDialogProps { + config: Record<string, unknown>; + info: Record<string, unknown>; + callback: (config: Record<string, unknown>, action: string) => void; + hide: () => void; +} + -function OptionsInputDialogComponent({ config, info, callback, hide }) { +function OptionsInputDialogComponent({ config, info, callback, hide }: OptionsInputDialogProps) {
21-26: Accessing class component state directly via ref.The pattern of accessing
optionsRef.current.state.configcreates tight coupling with OptionsForm's internal state structure. This works but is fragile if OptionsForm is refactored. Consider adding a getter method likegetConfig()to OptionsForm for a cleaner API in future refactoring.plugins/ohifv3/extensions/monai-label/src/components/SettingsTable.tsx (1)
52-52: Consider removing debug console.log statements.Console.log statements at Lines 52 and 59 may be debug artifacts. If these are not needed for production monitoring, consider removing them or using a proper logging framework.
Also applies to: 59-59
plugins/ohifv3/modes/monai-label/package.json (1)
21-25: Consider updating the minimum Node.js version requirement.Node.js 14 reached end-of-life in April 2023. While not blocking, consider updating the
enginesfield to require Node.js 18+ (current LTS) for better security and modern feature support.plugins/ohifv3/modes/monai-label/src/initToolGroups.ts (2)
54-55: Accessing private_servicesManagerproperty.Accessing
extensionManager._servicesManagerrelies on an internal/private property (underscore prefix convention). This creates coupling to implementation details that may change. Consider checking if there's a public API to access the services manager.
34-34: Hardcoded tool name'ProbeMONAILabel'.Unlike other tools that use
toolNames.X,'ProbeMONAILabel'is hardcoded as a string literal. This is likely a custom MONAI Label tool not in the standard Cornerstone toolNames. Consider adding a comment to clarify this is a custom tool, or define it in a constants file for consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
plugins/ohifv3/extensions/monai-label/package.jsonplugins/ohifv3/extensions/monai-label/src/components/OptionsInputDialog.tsxplugins/ohifv3/extensions/monai-label/src/components/SettingsTable.tsxplugins/ohifv3/modes/monai-label/babel.config.jsplugins/ohifv3/modes/monai-label/package.jsonplugins/ohifv3/modes/monai-label/src/index.tsxplugins/ohifv3/modes/monai-label/src/initToolGroups.tsplugins/ohifv3/modes/monai-label/src/toolbarButtons.jsplugins/ohifv3/modes/monai-label/src/toolbarButtons.ts
💤 Files with no reviewable changes (1)
- plugins/ohifv3/modes/monai-label/src/toolbarButtons.js
🧰 Additional context used
🧬 Code graph analysis (4)
plugins/ohifv3/extensions/monai-label/src/components/SettingsTable.tsx (2)
plugins/ohif/monai-label/src/components/SettingsTable.js (1)
SettingsTable(20-100)plugins/ohifv3/extensions/monai-label/src/utils/GenericUtils.js (1)
CookieUtils(93-143)
plugins/ohifv3/modes/monai-label/src/initToolGroups.ts (2)
plugins/ohif/monai-label/src/init.js (1)
tools(32-40)plugins/ohif/monai-label/src/commandsModule.js (1)
servicesManager(15-15)
plugins/ohifv3/modes/monai-label/src/index.tsx (2)
plugins/ohifv3/extensions/monai-label/src/utils/SegUtils.js (2)
segmentation(20-20)segmentation(21-21)plugins/ohif/monai-label/src/commandsModule.js (1)
servicesManager(15-15)
plugins/ohifv3/extensions/monai-label/src/components/OptionsInputDialog.tsx (2)
plugins/ohifv3/extensions/monai-label/src/components/actions/OptionsForm.tsx (1)
OptionsForm(20-211)plugins/ohifv3/extensions/monai-label/src/utils/SegUtils.js (1)
info(15-15)
🔇 Additional comments (13)
plugins/ohifv3/extensions/monai-label/src/components/OptionsInputDialog.tsx (2)
39-63: Clean migration to @ohif/ui-next Dialog components.The dialog structure follows the new component-based pattern well:
- Proper use of
onOpenChangefor handling ESC/backdrop dismissal.- Clear separation of header, content, and footer sections.
- Appropriate button variants (outline for secondary actions, default for primary).
69-79: No changes needed. OHIF'suiDialogService.show()automatically injects thehidefunction into the dialog content component as a prop, so theOptionsInputDialogComponentwill receive it despite it not being explicitly included incontentProps. The code correctly follows OHIF's standard dialog pattern.Likely an incorrect or invalid review comment.
plugins/ohifv3/extensions/monai-label/src/components/SettingsTable.tsx (2)
14-18: Successfully migrated to @ohif/ui-next.The migration from
@ohif/uito@ohif/ui-nextand the TypeScript conversion look good. The newIcons.ByNameAPI is correctly applied.
84-84: No action needed. The icon name"tool-reset"is valid in @ohif/ui-next and follows the documented legacy kebab-case naming convention forIcons.ByName. This icon is already used in other parts of the codebase (toolbarButtons.ts) with no issues.plugins/ohifv3/modes/monai-label/package.json (1)
37-38: LGTM!The peer dependency update to
@ohif/core: 3.11.0aligns with the PR objective to upgrade to OHIF v3.11.plugins/ohifv3/extensions/monai-label/package.json (1)
36-47: LGTM!The peer dependency updates to OHIF 3.11.0 are consistent across all packages and align with the PR objective.
plugins/ohifv3/modes/monai-label/src/index.tsx (2)
56-64: LGTM on the new toolbar API migration.The migration to
toolbarService.register()andupdateSection()follows the new OHIF v3.11 toolbar API pattern correctly. The section organization (primary, viewportActionMenu areas, MoreTools) is well-structured.
112-128: Thorough service cleanup on mode exit.Good practice to properly destroy all services when exiting the mode to prevent memory leaks and state pollution.
plugins/ohifv3/modes/monai-label/src/toolbarButtons.ts (2)
340-349: Verify intentional omission ofevaluateproperty.Unlike other tool buttons,
TagBrowserlacks anevaluateproperty. If this is intentional (tool should always be available), consider adding a comment for clarity. Otherwise, add an appropriate evaluate strategy for consistency.
4-9: Good reuse of command configuration.The shared
setToolActiveToolbarobject reduces duplication across button definitions. The button configurations with proper typing and evaluate strategies are well-structured.Also applies to: 11-27
plugins/ohifv3/modes/monai-label/src/initToolGroups.ts (3)
25-30: Verify intentional dual registration ofStackScroll.
StackScrollappears in bothactive(with bindings for wheel/touch) andpassivearrays. This may be intentional to support different interaction modes, but if not, it could lead to unexpected behavior.
72-85: Good defensive fallback for missing viewport.The
getReferenceLineColorcallback properly handles the case when viewport info is unavailable by logging a warning and returning a default color, preventing runtime errors.
120-124: LGTM on the overall tool group initialization structure.The
initToolGroupsfunction properly orchestrates the initialization of default, MPR, and volume3D tool groups with appropriate configurations for each context.
| "@cornerstonejs/adapters": "^3.32.5", | ||
| "@cornerstonejs/core": "^3.32.5", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@cornerstonejs/core version 3.0 breaking changes
💡 Result:
Short summary of notable breaking changes in @cornerstonejs/core → 3.0 (migration highlights):
-
Voxel / volume API: introduction of VoxelManager and changed access to volume/voxel properties (no large vtk scalar arrays; use properties like numberOfComponents via new APIs). This affects how you read/modify volume data. [1]
-
Image/volume loading API changes:
- createLocalVolume / derived-volume functions signature/behavior changed (parameter order, synchronous vs Promise for some derived helpers). Several volume helper names were renamed (e.g., derived segmentation → labelmap). [1]
-
Caching / cache API changes:
- Image-only caching model (volume-specific cache size removed).
- New synchronous cache methods (putImageSync / putVolumeSync) and changed isCacheable behaviour. [1]
-
GPU detection / init flags:
- Removal of detect-gpu dependency and related detectGPUConfig; default GPU tier used and gpu tier is now configurable via init. [1]
-
Target buffer / data-type options:
- targetBufferType replaced by a targetBuffer object; some 16-bit flags / use16BitDataType removed/changed. [1]
-
Adapters & Measurements API:
- MeasurementsReport and adapter registration APIs changed (maps vs objects, new base class, new init/register methods). Annotation events and coordinate handling updated (some events no longer require viewport id; world/image coord helpers moved to utilities). [2]
-
Tooling and package-level changes:
- Major surface/segmentation and tools changes aligned with Cornerstone3D v3.0 — expect breaking changes in packages that depend on @cornerstonejs/core (see relevant package changelogs). [3]
What to do next
- Read the 3.x migration guides and adapters docs on the Cornerstone site and review package changelogs for @cornerstonejs/tools and related packages before upgrading. Key docs: migration and adapters pages (links below). [1][2][3]
Sources
- Cornerstone migration docs (volume/voxel, loaders, cache, init, target buffer, etc.). [1]
- Cornerstone 3.x adapters / MeasurementsReport migration notes. [2]
- @cornerstonejs/tools changelog (3.0 release notes / dependent package changes). [3]
References
[1] https://www.cornerstonejs.org/docs/migration-guides/2x/core (migration notes covering volume/voxel, loaders, cache, gpu/init and related breaking changes).
[2] https://www.cornerstonejs.org/docs/migration-guides/3x/adapters/ (adapters & MeasurementsReport changes).
[3] @cornerstonejs/tools changelog / release notes (3.0).
🏁 Script executed:
# First, let's examine the package.json file to verify the duplicate dependency claim
cat -n plugins/ohifv3/extensions/monai-label/package.json | head -100Repository: Project-MONAI/MONAILabel
Length of output: 2886
🏁 Script executed:
# Search for all occurrences of @cornerstonejs packages in the file
rg "@cornerstonejs" plugins/ohifv3/extensions/monai-label/package.json -A 0 -B 0Repository: Project-MONAI/MONAILabel
Length of output: 290
🏁 Script executed:
# Search for imports and usage of Cornerstone packages in the source code
rg "@cornerstonejs" plugins/ohifv3/extensions/monai-label/src -t ts -t tsx -t js -t jsx --max-count 20Repository: Project-MONAI/MONAILabel
Length of output: 95
🏁 Script executed:
# Check if Cornerstone is used in runtime code or only imported for types/build
rg "cornerstonejs" plugins/ohifv3/extensions/monai-label/src -B 2 -A 2 -t ts -t tsxRepository: Project-MONAI/MONAILabel
Length of output: 95
🏁 Script executed:
# Search for Cornerstone imports and usage without file type restrictions
rg "@cornerstonejs" plugins/ohifv3/extensions/monai-label/src --max-count 30Repository: Project-MONAI/MONAILabel
Length of output: 1076
🏁 Script executed:
# Also search in the entire monai-label extension directory
find plugins/ohifv3/extensions/monai-label/src -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) | head -20Repository: Project-MONAI/MONAILabel
Length of output: 1488
Verify compatibility with @cornerstonejs v3.x breaking changes and remove duplicate dependencies.
Breaking changes in @cornerstonejs/core v3.0 include significant API updates to volume/voxel handling, image loading, caching, adapters, and tools. The codebase uses core APIs (cache, triggerEvent, eventTarget) and tools extensively in runtime components—verify these usages are compatible with v3.x.
Additionally, @cornerstonejs/adapters and @cornerstonejs/core are listed in both dependencies and devDependencies. Since they are runtime dependencies (used in .tsx component files), remove them from devDependencies. Also note that @cornerstonejs/tools is in devDependencies but is used at runtime—it should be moved to dependencies.
Also applies to: 66-68
| "@kitware/vtk.js": "32.12.0", | ||
| "react-color": "^2.19.3", | ||
| "md5.js": "^1.3.5", | ||
| "axios": "^0.21.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
axios CVE-2021-3749 ReDoS vulnerability
💡 Result:
Summary: CVE-2021-3749 is a regular-expression denial-of-service (ReDoS) in axios (affecting versions < 0.21.2). The vulnerable trim/regex can be triggered by crafted input to cause excessive CPU consumption (CVSS ~7.5). Fixed in axios v0.21.2 — upgrade to >=0.21.2 (or later) to remediate. [1][2]
References
- NVD entry for CVE-2021-3749. [1]
- Advisory / vulnerability pages noting affected versions and patch (axios < 0.21.2; fixed in 0.21.2). [2]
🌐 Web query:
axios latest stable version 2024 2025 security
💡 Result:
Short answer
- As of the 2025 releases list, the axios stable line is ahead of 1.8.2 — the release tracker shows v1.13.2 (Nov 4, 2025) as a recent stable release. [1]
- Important security: CVE-2025-27152 (SSRF / credential leakage when absolute URLs are used despite baseURL) was disclosed Mar 7, 2025 and is fixed in axios v1.8.2 (and 0.30.0). If you run any affected version (<1.8.2 or <0.30.0), upgrade. [2][3]
Actionable recommendations
- Upgrade to the latest stable axios available in your environment (>=1.8.2). Prefer the most recent stable release (e.g., 1.13.2 as of Nov 4, 2025) if compatible. [1][2]
- Audit code that uses baseURL/create() and validate/normalize any input URLs (reject untrusted absolute URLs) to avoid SSRF/credential leakage. See the GitHub advisory and CVE for details. [2][3]
Sources
- Release list showing recent axios versions (includes v1.13.2). [turn1search2].
- GitHub security advisory for axios (GHSA‑jr5f‑v2jv‑69x6) — CVE-2025-27152, patched in 1.8.2 / 0.30.0. [turn0search6].
- Ubuntu / NVD CVE entry for CVE-2025-27152 (publication Mar 7, 2025). [turn0search3].
🌐 Web query:
[email protected] release security fixes compared to 0.21.1
💡 Result:
Short answer: axios 1.7.0 is newer than 0.21.1 but does NOT contain fixes for several security issues that affect older 0.21.x releases; several important vulnerabilities (ReDoS, data: URL DoS, and SSRF fixes) were fixed in later 1.8.x / 1.12.x releases — so 1.7.0 is safer than 0.21.1 for some historical bugs but remains vulnerable to a number of known issues. See details and recommended upgrade versions below.
What changed / notable security items
- 0.21.0 → 0.21.1: 0.21.1 fixed a proxy/redirect SSRF bug present in 0.21.0 (so 0.21.1 is safer than 0.21.0 for that issue). [1][4]
- 0.21.1 known problems: older 0.21.1 is affected by multiple vulnerabilities (ReDoS, CSRF/ XSRF header issues, DoS via data: URIs, etc.) that were patched in later 0.21.x / 0.28.x / 1.x lines. See Snyk / vulnerability databases for specifics. [1][2]
- 1.7.0 release notes: 1.7.0 (May 19, 2024) is mainly a feature/bugfix release (fetch adapter + bug fixes) — its changelog does not list security fixes for the newer SSRF / data: DoS / ReDoS issues. [3]
- Known vulnerabilities remaining in 1.7.0: vulnerability trackers show SSRF, data: URL allocation (DoS) and some ReDoS issues affecting many 1.x releases up through 1.7.x; fixes for these were introduced in later 1.8.2/1.8.3/1.8.4 and in higher 1.12.x releases depending on the issue. For example CVE-2025-27152 (SSRF/absolute URL handling) is fixed in 1.8.2+; the data: URL allocation (DoS) and other ReDoS issues were fixed in later 1.12.0+ / 1.7.4+ depending on the specific CVE. [4][2][5]
Recommendations
- Do not assume 1.7.0 is fully secure relative to 0.21.1. Upgrade to a version that explicitly includes the security fixes you need:
- For the absolute-URL/SSRF fix: upgrade to >= 1.8.2 / 1.8.3 (depending on the advisory). [4]
- For the data: URL allocation (DoS) and some ReDoS fixes: upgrade to >= 1.12.0 (or the specific patched minor/patch noted by your vulnerability scanner). [2][5]
- Run your dependency scanner (Snyk/OSV/your SCA) against your codebase to identify which CVEs affect you and which minimum axios version patches them.
Sources
- Snyk vulnerabilities listing for axios 0.21.1 (vulnerabilities and recommended fixed versions). [1]
- Meterian / vulnerability listings summarizing CVEs affecting 0.21.1 and 1.7.0. [2]
- axios changelog / release notes for 1.7.0. [3]
- Debian / OSV advisory (CVE-2025-27152) and fixed-version info (fixed in 1.8.2+). [4]
- Snyk vulnerabilities listing for axios 1.7.0 (shows remaining issues and upgrade targets). [5]
If you want, I can list the exact CVE IDs that affect 0.21.1 and whether each one is fixed in 1.7.0 or which exact version fixes it.
Security: Upgrade axios to address known vulnerabilities.
axios@^0.21.1 has CVE-2021-3749 (ReDoS) and other security vulnerabilities. Upgrade to axios@^1.8.2 or later (current stable is 1.13.2). Version 1.7.0 alone is insufficient as it still contains multiple known vulnerabilities fixed in later releases.
🤖 Prompt for AI Agents
In plugins/ohifv3/extensions/monai-label/package.json around line 55, the axios
dependency is pinned to "^0.21.1" which is vulnerable; update the dependency to
a secure, supported version (use "axios": "^1.8.2" or preferably the current
stable like "^1.13.2") in package.json, run your package manager to regenerate
the lockfile (npm/yarn/pnpm install), resolve any breaking API changes in code
that imports axios, run the test suite and linting to verify nothing broke, and
commit both the updated package.json and updated lockfile.
| import { CookieUtils } from '../utils/GenericUtils'; | ||
|
|
||
| export default class SettingsTable extends Component { | ||
| onInfo: any; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any with proper function signature.
The any type annotation defeats TypeScript's type safety. Based on usage at Line 60, this should be typed as a function that accepts a URL string.
🔎 Suggested fix
- onInfo: any;
+ onInfo: (url: string) => void;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onInfo: any; | |
| onInfo: (url: string) => void; |
🤖 Prompt for AI Agents
In plugins/ohifv3/extensions/monai-label/src/components/SettingsTable.tsx around
line 21, the prop `onInfo` is currently typed as `any`; change its type to a
function signature that accepts a URL string (e.g. onInfo: (url: string) =>
void) so TypeScript enforces correct usage where the prop is called with a URL
at line 60.
| onConnect = () => { | ||
| const url = document.getElementById('monailabelServerURL').value; | ||
| this.setState({ url: url }); | ||
| CookieUtils.setCookie('MONAILABEL_SERVER_URL', url); | ||
| console.log('Connecting Server', url); | ||
| this.onInfo(url); | ||
| }; |
There was a problem hiding this comment.
Avoid direct DOM manipulation in React.
Line 56 uses document.getElementById() to read the input value, which breaks React's declarative paradigm. Since onBlurSeverURL already updates this.state.url, you can use the state value directly.
🔎 Recommended fix
onConnect = () => {
- const url = document.getElementById('monailabelServerURL').value;
- this.setState({ url: url });
- CookieUtils.setCookie('MONAILABEL_SERVER_URL', url);
- console.log('Connecting Server', url);
- this.onInfo(url);
+ console.log('Connecting Server', this.state.url);
+ this.onInfo(this.state.url);
};The state and cookie updates are already handled by onBlurSeverURL.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In plugins/ohifv3/extensions/monai-label/src/components/SettingsTable.tsx around
lines 55–61, the onConnect handler reads the input with
document.getElementById(...) which bypasses React state; instead use the
already-updated this.state.url (or read from props/state) and remove the
redundant setState/CookieUtils.setCookie calls here since onBlurSeverURL handles
updating state and cookies; call this.onInfo(this.state.url) (or await a state
update if you must set state here) and remove direct DOM access to keep the
component declarative.
| @@ -0,0 +1,403 @@ | |||
| import type { Button } from '@ohif/core/types'; | |||
| import { ViewportGridService } from '@ohif/core'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused import.
ViewportGridService is imported but never used in this file.
🔎 Proposed fix
import type { Button } from '@ohif/core/types';
-import { ViewportGridService } from '@ohif/core';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ViewportGridService } from '@ohif/core'; | |
| import type { Button } from '@ohif/core/types'; |
🤖 Prompt for AI Agents
In plugins/ohifv3/modes/monai-label/src/toolbarButtons.ts around line 2, the
file imports ViewportGridService from '@ohif/core' but never uses it; remove the
unused import line (or, if the service is intended to be used, replace the
import with the actual usage) so the import list contains only used modules and
no linting/unused-import warnings.
2 participants
feat: Upgrade OHIF to v3.11 with new UI components and services
This pull request upgrades our application to support the latest features and architectural improvements in OHIF v3.11. The update focuses on modernizing the UI and integrating new, centralized services for a more modular and consistent user experience.
Key Changes & Features
@ohif/ui-nextpackage.ToolbarService: The legacy toolbar system has been replaced with the newToolbarServiceAPI.modalityLoadBadgetrackingStatusnavigationComponentValidation Status & Scope
Limited Testing Scope: Due to the scope of this PR, comprehensive testing across all models was not performed.
Functionality Verified: The segmentation functionality has been verified and is confirmed to be working as expected.
Please review it
@sedghi @SachidanandAlle
Summary by CodeRabbit
Refactor
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.