Skip to content

test#5778

Open
sedghi wants to merge 1 commit intomasterfrom
test/greptiles
Open

test#5778
sedghi wants to merge 1 commit intomasterfrom
test/greptiles

Conversation

@sedghi
Copy link
Member

@sedghi sedghi commented Feb 4, 2026

Context

Changes & Results

Testing

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:
  • [] Node version:
  • [] Browser:

Note

High Risk
Removes the getImageId helper from a core DICOM loading utility, but dicomLoaderService.js still calls getImageId, which is likely to cause runtime failures when resolving image IDs and retrieving DICOM data.

Overview
Removes the local getImageId helper from extensions/cornerstone/src/utils/dicomLoaderService.js, changing how image IDs are expected to be resolved.

Note: the file still calls getImageId (e.g., in findImageIdOnStudies and getImageInstanceId) without defining or importing it, which likely breaks DICOM retrieval at runtime.

Written by Cursor Bugbot for commit e4c4df0. This will update automatically on new commits. Configure here.

Greptile Overview

Greptile Summary

This PR removes the getImageId helper function from dicomLoaderService.js without removing its usages, causing critical runtime errors.

  • Critical Issue: Function getImageId deleted but still called on lines 17 and 36
  • Impact: Any code path invoking findImageIdOnStudies or getImageInstanceId will throw ReferenceError: getImageId is not defined
  • Affected flows: Local DICOM data loading and image instance ID retrieval

Confidence Score: 0/5

  • This PR is not safe to merge - it contains critical bugs that will cause runtime errors
  • Score of 0 reflects that the removed function is still referenced in 2 locations, which will cause immediate ReferenceError exceptions when those code paths execute
  • extensions/cornerstone/src/utils/dicomLoaderService.js requires immediate attention to restore the deleted function or inline its logic

Important Files Changed

Filename Overview
extensions/cornerstone/src/utils/dicomLoaderService.js Removed getImageId helper function but left 2 call sites, causing ReferenceError

Sequence Diagram

sequenceDiagram
    participant Client
    participant DicomLoaderService
    participant findImageIdOnStudies
    participant getImageInstanceId
    participant getImageId
    
    Client->>DicomLoaderService: findDicomDataPromise(dataset, studies)
    DicomLoaderService->>DicomLoaderService: getLocalData(dataset, studies)
    DicomLoaderService->>getImageInstanceId: getImageInstanceId(instance)
    getImageInstanceId->>getImageId: getImageId(imageInstance)
    Note over getImageId: ❌ DELETED FUNCTION<br/>Still called on line 36
    getImageId-->>getImageInstanceId: ReferenceError
    
    DicomLoaderService->>findImageIdOnStudies: findImageIdOnStudies(studies, displaySetInstanceUID)
    findImageIdOnStudies->>getImageId: getImageId(instance)
    Note over getImageId: ❌ DELETED FUNCTION<br/>Still called on line 17
    getImageId-->>findImageIdOnStudies: ReferenceError
Loading

(2/5) Greptile learns from your feedback when you react with thumbs up/down!

Copilot AI review requested due to automatic review settings February 4, 2026 18:14
@netlify
Copy link

netlify bot commented Feb 4, 2026

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit e4c4df0
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/69838c949c9b5900087289c2
😎 Deploy Preview https://deploy-preview-5778--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.


return typeof imageObj.getImageId === 'function' ? imageObj.getImageId() : imageObj.url;
};

Copy link

Choose a reason for hiding this comment

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

Removed function still called causing runtime crash

High Severity

The getImageId function was removed from this file, but it's still called at two locations: in findImageIdOnStudies (returning getImageId(instance)) and in getImageInstanceId (returning getImageId(imageInstance)). Any code path reaching these functions will crash with a ReferenceError. The DICOM loader service will be completely broken for local data retrieval and image type detection workflows.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Additional Comments (2)

extensions/cornerstone/src/utils/dicomLoaderService.js
getImageId function was deleted but is still called here - will cause ReferenceError: getImageId is not defined at runtime

  return instance?.getImageId?.() ?? instance?.url;
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/cornerstone/src/utils/dicomLoaderService.js
Line: 17:17

Comment:
`getImageId` function was deleted but is still called here - will cause `ReferenceError: getImageId is not defined` at runtime

```suggestion
  return instance?.getImageId?.() ?? instance?.url;
```

How can I resolve this? If you propose a fix, please make it concise.

extensions/cornerstone/src/utils/dicomLoaderService.js
getImageId function was deleted but is still called here - will cause ReferenceError: getImageId is not defined at runtime

  return imageInstance?.getImageId?.() ?? imageInstance?.url;
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/cornerstone/src/utils/dicomLoaderService.js
Line: 36:36

Comment:
`getImageId` function was deleted but is still called here - will cause `ReferenceError: getImageId is not defined` at runtime

```suggestion
  return imageInstance?.getImageId?.() ?? imageInstance?.url;
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes a getImageId helper function from the dicomLoaderService utility file. However, the PR appears to be incomplete and contains a critical bug.

Changes:

  • Removed the getImageId helper function (lines 6-14) from dicomLoaderService.js

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -3,14 +3,6 @@ import dicomImageLoader from '@cornerstonejs/dicom-image-loader';
import { api } from 'dicomweb-client';
import { DICOMWeb, errorHandler } from '@ohif/core';

Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

Removing the getImageId function will break the code. This function is still referenced in two places within this file:

  1. In findImageIdOnStudies (line 17 in the updated file): return getImageId(instance);
  2. In getImageInstanceId (line 36 in the updated file): return getImageId(imageInstance);

These references will cause ReferenceError at runtime when either function is called. Before removing this function, you need to either:

  • Import a replacement from another module (if one exists in @ohif/core utils)
  • Inline the function logic at the call sites
  • Keep this function definition
Suggested change
const getImageId = instance => {
return instance && instance.imageId;
};

Copilot uses AI. Check for mistakes.
@cypress
Copy link

cypress bot commented Feb 4, 2026

Viewers    Run #5906

Run Properties:  status check passed Passed #5906  •  git commit e4c4df01eb: test
Project Viewers
Branch Review test/greptiles
Run status status check passed Passed #5906
Run duration 02m 33s
Commit git commit e4c4df01eb: test
Committer Alireza
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 37
View all changes introduced in this branch ↗︎

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