Auto Seg - fix loading from/into patient dictionary and patch ROI numbers

[Timmy-Codes]

Updated RTStructFileLoader.py to load new rtss dataset into the patient dictionary - was inhibiting DVH calculations of segments
Refactored NiftiToRtstructConverter.py to load rtss from patient dictionary
Add ROI number patch to fix/avoid "overwrite" when existing ROI numbers are greater than the length of the StructureSetROISequence
Added check for existing rtstruct file in NiftiToRtstructConverter.py
Move Nifti file clean up to ensure proper removal of segmentation files

Summary by Sourcery

Refine auto-segmentation pipeline to better integrate RTStruct files via the patient dictionary, prevent ROI numbering collisions, and ensure proper cleanup and messaging.

Enhancements:

• Refactor RTStruct conversion to load and save files from the patient dictionary container with a fallback to creating new RTStructs
• Add logic to compute and assign unique ROI numbers for newly added structures to avoid overwriting existing ROIs
• Enhance RTStructFileLoader to store the parsed RTStruct dataset in the patient dictionary for downstream DVH calculations
• Move Nifti file cleanup into the workflow’s final block to guarantee removal of temporary segmentation files
• Update segmentation success dialog text to reference the RTStruct file location and fix logging error formatting for fusion overlay updates

[sourcery-ai]

Reviewer's Guide

This PR refactors RTStruct handling by loading and saving from a patient dictionary, patches ROI numbering to avoid duplicates, adds existence checks for RTStruct files, reorders cleanup steps for robust file removal, and updates UI messaging and logging formatting.

Sequence diagram for loading and saving RTStruct via patient dictionary

sequenceDiagram
participant "AutoSegmentation"
participant "PatientDictContainer"
participant "RTStructBuilder"
participant "RTStructFileLoader"

"AutoSegmentation"->>"PatientDictContainer": Prepare output paths
"AutoSegmentation"->>"RTStructBuilder": Convert Nifti to RTStruct
alt RTStruct file exists in patient dictionary
    "RTStructBuilder"->>"PatientDictContainer": Load rtss path
    "RTStructBuilder"->>"RTStructBuilder": Load existing RTStruct from rtss path
else RTStruct file does not exist
    "RTStructBuilder"->>"RTStructBuilder": Create new RTStruct
end
"RTStructBuilder"->>"PatientDictContainer": Save RTStruct to rtss path
"RTStructFileLoader"->>"PatientDictContainer": Load rtss file to patient dictionary
"RTStructFileLoader"->>"PatientDictContainer": Set dataset['rtss'] with dcmread(rtss_path)

Loading

ER diagram for RTStruct ROI number patching

erDiagram
  RTStruct {
    ROINumber int
    StructureSetROISequence list
    ROIContourSequence list
    RTROIObservationsSequence list
  }
  StructureSetROISequence {
    ROINumber int
  }
  ROIContourSequence {
    ReferencedROINumber int
  }
  RTROIObservationsSequence {
    ReferencedROINumber int
  }
  RTStruct ||--o{ StructureSetROISequence : contains
  RTStruct ||--o{ ROIContourSequence : contains
  RTStruct ||--o{ RTROIObservationsSequence : contains
  StructureSetROISequence }o--|| RTStruct : references
  ROIContourSequence }o--|| RTStruct : references
  RTROIObservationsSequence }o--|| RTStruct : references

Loading

Class diagram for PatientDictContainer and RTStructBuilder changes

classDiagram
class PatientDictContainer {
  +filepaths: dict
  +dataset: dict
  +set(key, value)
}
class RTStructBuilder {
  +create_from(rt_struct_path, dicom_series_path)
  +create_new(dicom_series_path)
  +save(path)
  +add_roi(mask, name)
}
PatientDictContainer "1" o-- "1" RTStructBuilder : uses rtss path

Loading

File-Level Changes

Change	Details	Files
Patch ROI numbering to avoid duplicates	Compute next available ROINumber from existing Sequence entries Assign new ROINumber to StructureSetROISequence, ROIContourSequence, RTROIObservationsSequence	src/Model/NiftiToRtstructConverter.py
Load/save RTStruct via patient dictionary	Use PatientDictContainer to retrieve or set rtss file path Conditionally load existing RTStruct from container or create new Save RTStruct output back to patient dictionary path	src/Model/NiftiToRtstructConverter.py
Integrate RTStruct dataset into patient dictionary	Read RTStruct file with dcmread and store in patient_dict_container.dataset['rtss'] Add error handling for failed file read	src/Controller/RTStructFileLoader.py
Reorder cleanup for robust segmentation file removal	Move _cleanup_nifti_dir call into finally block Ensure temporary directories are cleaned after workflow completion	src/Model/AutoSegmentation/AutoSegmentation.py
Update UI messaging and logging formatting	Adjust success dialog text to refer to RTStruct file dynamically Correct logging.error call to use f-string for exception message	src/View/AutoSegmentation/AutoSegmentWindow.py src/View/mainpage/WindowingSlider.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• The converter sets a new rtss_path when creating a fresh RTStruct but doesn’t update patient_dict_container.filepaths['rtss'], so the patient dictionary may still point to the old path—please sync the container path after creation.
• Before patching the last ROI’s ROINumber, add guards to ensure StructureSetROISequence, ROIContourSequence, and RTROIObservationsSequence exist and contain elements to avoid potential index errors.
• Consider renaming rtss_dataset to something more descriptive (e.g. rtstruct_ds) to clearly distinguish between the DICOM dataset object and the rtss file path variable.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The converter sets a new rtss_path when creating a fresh RTStruct but doesn’t update patient_dict_container.filepaths['rtss'], so the patient dictionary may still point to the old path—please sync the container path after creation.
- Before patching the last ROI’s ROINumber, add guards to ensure StructureSetROISequence, ROIContourSequence, and RTROIObservationsSequence exist and contain elements to avoid potential index errors.
- Consider renaming rtss_dataset to something more descriptive (e.g. rtstruct_ds) to clearly distinguish between the DICOM dataset object and the rtss file path variable.

## Individual Comments

### Comment 1
<location> `src/Model/AutoSegmentation/AutoSegmentation.py:66-75` </location>
<code_context>
             self._copy_temp_dicom_dir()
             self._run_totalsegmentation(task, roi_subset, output_dir)
             self._convert_to_rtstruct(output_dir, output_rt)
-            self._cleanup_nifti_dir(output_dir)
             self.signals.finished.emit()
         except Exception as e:
</code_context>

<issue_to_address>
**suggestion:** Moving cleanup to finally block may remove files needed for error analysis.

Consider making cleanup conditional or logging the output directory location before removal to support error analysis after failures.

```suggestion
            self._copy_temp_dicom_dir()
            self._run_totalsegmentation(task, roi_subset, output_dir)
            self._convert_to_rtstruct(output_dir, output_rt)
            self.signals.finished.emit()
            cleanup_output_dir = True
        except Exception as e:
            self.signals.error.emit(str(e))
            logger.exception("Segmentation workflow failed")
            logger.info(f"Segmentation failed. Output directory retained for error analysis: {output_dir}")
            cleanup_output_dir = False
        finally:
            if cleanup_output_dir:
                self._cleanup_nifti_dir(output_dir)
            self._cleanup_temp_dir()
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sjswerdloff]

Excellent work. Surgical fixes.

[AJB-BigA]

Lgtm

[Nowhere4Nothing]

Legend, looks good 🤘