fix(pipeline): copy input files to results_dir and add UI cleanup
Browse filesAddresses two additional issues discovered during pre-flight verification:
P0 FIX - UI crash on file access:
- run_pipeline_on_case() returned input_files pointing to HF temp files
that were deleted when the dataset context manager exited
- UI would crash with FileNotFoundError when trying to read DWI for viz
- Solution: Copy DWI and ADC to results_dir before context exit
- All paths in PipelineResult now remain valid after function returns
P1 FIX - Results directory accumulation:
- When output_dir=None, pipeline creates temp directory for results
- This directory was never cleaned up, causing slow disk accumulation
- Solution: UI now tracks previous results_dir and cleans up on next run
- Added results_dir field to PipelineResult for caller cleanup control
API changes:
- PipelineResult.staged_dir renamed to results_dir
- PipelineResult.input_files now contains copied paths (always valid)
- Results directory cleanup is caller's responsibility (UI handles this)
Test updates:
- All fixtures now use real temp files (pipeline copies them)
- Updated field name references (staged_dir -> results_dir)
All 125 tests pass, ruff and mypy clean.
- src/stroke_deepisles_demo/pipeline.py +31 -8
- src/stroke_deepisles_demo/ui/app.py +18 -1
- tests/test_cli.py +2 -2
- tests/test_pipeline.py +21 -22
- tests/test_pipeline_cleanup.py +10 -5
- tests/ui/test_app.py +1 -1
|
@@ -25,11 +25,15 @@ logger = get_logger(__name__)
|
|
| 25 |
|
| 26 |
@dataclass(frozen=True)
|
| 27 |
class PipelineResult:
|
| 28 |
-
"""Complete result of running the pipeline on a case.
|
|
|
|
|
|
|
|
|
|
|
|
|
| 29 |
|
| 30 |
case_id: str
|
| 31 |
-
input_files: CaseFiles
|
| 32 |
-
|
| 33 |
prediction_mask: Path
|
| 34 |
ground_truth: Path | None
|
| 35 |
dice_score: float | None # None if ground truth unavailable or not computed
|
|
@@ -110,15 +114,34 @@ def run_pipeline_on_case(
|
|
| 110 |
# Stage files (copies DWI/ADC to staging directory)
|
| 111 |
staged = stage_case_for_deepisles(case_files, staging_root)
|
| 112 |
|
| 113 |
-
# Copy
|
| 114 |
-
# (HuggingFace mode stores
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 115 |
ground_truth: Path | None = None
|
| 116 |
original_ground_truth = case_files.get("ground_truth")
|
| 117 |
if original_ground_truth and original_ground_truth.exists():
|
| 118 |
-
results_dir.mkdir(parents=True, exist_ok=True)
|
| 119 |
ground_truth = results_dir / f"{resolved_case_id}_ground_truth.nii.gz"
|
| 120 |
shutil.copy2(original_ground_truth, ground_truth)
|
| 121 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 122 |
# Dataset temp files cleaned up here (context manager __exit__)
|
| 123 |
|
| 124 |
# 3. Run Inference
|
|
@@ -145,8 +168,8 @@ def run_pipeline_on_case(
|
|
| 145 |
|
| 146 |
return PipelineResult(
|
| 147 |
case_id=resolved_case_id,
|
| 148 |
-
input_files=
|
| 149 |
-
|
| 150 |
prediction_mask=inference_result.prediction_path,
|
| 151 |
ground_truth=ground_truth,
|
| 152 |
dice_score=dice_score,
|
|
|
|
| 25 |
|
| 26 |
@dataclass(frozen=True)
|
| 27 |
class PipelineResult:
|
| 28 |
+
"""Complete result of running the pipeline on a case.
|
| 29 |
+
|
| 30 |
+
All file paths in this result point to valid, accessible files in results_dir.
|
| 31 |
+
Callers are responsible for cleaning up results_dir when done (if desired).
|
| 32 |
+
"""
|
| 33 |
|
| 34 |
case_id: str
|
| 35 |
+
input_files: CaseFiles # Copied to results_dir; always valid paths
|
| 36 |
+
results_dir: Path # Directory containing all result files (for cleanup)
|
| 37 |
prediction_mask: Path
|
| 38 |
ground_truth: Path | None
|
| 39 |
dice_score: float | None # None if ground truth unavailable or not computed
|
|
|
|
| 114 |
# Stage files (copies DWI/ADC to staging directory)
|
| 115 |
staged = stage_case_for_deepisles(case_files, staging_root)
|
| 116 |
|
| 117 |
+
# Copy input files to results_dir before dataset cleanup
|
| 118 |
+
# (HuggingFace mode stores files in temp dirs that get cleaned up)
|
| 119 |
+
# This ensures all paths in PipelineResult remain valid after function returns
|
| 120 |
+
results_dir.mkdir(parents=True, exist_ok=True)
|
| 121 |
+
|
| 122 |
+
# Copy DWI (required for UI visualization)
|
| 123 |
+
dwi_dest = results_dir / f"{resolved_case_id}_dwi.nii.gz"
|
| 124 |
+
shutil.copy2(case_files["dwi"], dwi_dest)
|
| 125 |
+
|
| 126 |
+
# Copy ADC
|
| 127 |
+
adc_dest = results_dir / f"{resolved_case_id}_adc.nii.gz"
|
| 128 |
+
shutil.copy2(case_files["adc"], adc_dest)
|
| 129 |
+
|
| 130 |
+
# Copy ground truth if available
|
| 131 |
ground_truth: Path | None = None
|
| 132 |
original_ground_truth = case_files.get("ground_truth")
|
| 133 |
if original_ground_truth and original_ground_truth.exists():
|
|
|
|
| 134 |
ground_truth = results_dir / f"{resolved_case_id}_ground_truth.nii.gz"
|
| 135 |
shutil.copy2(original_ground_truth, ground_truth)
|
| 136 |
|
| 137 |
+
# Build input_files with copied paths (always valid after function returns)
|
| 138 |
+
preserved_input_files: CaseFiles = {
|
| 139 |
+
"dwi": dwi_dest,
|
| 140 |
+
"adc": adc_dest,
|
| 141 |
+
}
|
| 142 |
+
if ground_truth:
|
| 143 |
+
preserved_input_files["ground_truth"] = ground_truth
|
| 144 |
+
|
| 145 |
# Dataset temp files cleaned up here (context manager __exit__)
|
| 146 |
|
| 147 |
# 3. Run Inference
|
|
|
|
| 168 |
|
| 169 |
return PipelineResult(
|
| 170 |
case_id=resolved_case_id,
|
| 171 |
+
input_files=preserved_input_files,
|
| 172 |
+
results_dir=results_dir,
|
| 173 |
prediction_mask=inference_result.prediction_path,
|
| 174 |
ground_truth=ground_truth,
|
| 175 |
dice_score=dice_score,
|
|
@@ -2,7 +2,8 @@
|
|
| 2 |
|
| 3 |
from __future__ import annotations
|
| 4 |
|
| 5 |
-
|
|
|
|
| 6 |
|
| 7 |
import gradio as gr
|
| 8 |
from matplotlib.figure import Figure # noqa: TC002
|
|
@@ -20,8 +21,14 @@ from stroke_deepisles_demo.ui.viewer import (
|
|
| 20 |
render_slice_comparison,
|
| 21 |
)
|
| 22 |
|
|
|
|
|
|
|
|
|
|
| 23 |
logger = get_logger(__name__)
|
| 24 |
|
|
|
|
|
|
|
|
|
|
| 25 |
|
| 26 |
def run_segmentation(
|
| 27 |
case_id: str, fast_mode: bool, show_ground_truth: bool
|
|
@@ -47,6 +54,13 @@ def run_segmentation(
|
|
| 47 |
)
|
| 48 |
|
| 49 |
try:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 50 |
logger.info("Running segmentation for %s", case_id)
|
| 51 |
result = run_pipeline_on_case(
|
| 52 |
case_id,
|
|
@@ -55,6 +69,9 @@ def run_segmentation(
|
|
| 55 |
cleanup_staging=True,
|
| 56 |
)
|
| 57 |
|
|
|
|
|
|
|
|
|
|
| 58 |
# 1. NiiVue Visualization
|
| 59 |
# We need data URLs for the browser
|
| 60 |
# Note: This reads the file content into memory (base64)
|
|
|
|
| 2 |
|
| 3 |
from __future__ import annotations
|
| 4 |
|
| 5 |
+
import shutil
|
| 6 |
+
from typing import TYPE_CHECKING, Any
|
| 7 |
|
| 8 |
import gradio as gr
|
| 9 |
from matplotlib.figure import Figure # noqa: TC002
|
|
|
|
| 21 |
render_slice_comparison,
|
| 22 |
)
|
| 23 |
|
| 24 |
+
if TYPE_CHECKING:
|
| 25 |
+
from pathlib import Path
|
| 26 |
+
|
| 27 |
logger = get_logger(__name__)
|
| 28 |
|
| 29 |
+
# Shared output directory for UI results (cleaned up between runs to prevent disk accumulation)
|
| 30 |
+
_previous_results_dir: Path | None = None
|
| 31 |
+
|
| 32 |
|
| 33 |
def run_segmentation(
|
| 34 |
case_id: str, fast_mode: bool, show_ground_truth: bool
|
|
|
|
| 54 |
)
|
| 55 |
|
| 56 |
try:
|
| 57 |
+
global _previous_results_dir
|
| 58 |
+
|
| 59 |
+
# Clean up previous results to prevent disk accumulation on HF Spaces
|
| 60 |
+
if _previous_results_dir and _previous_results_dir.exists():
|
| 61 |
+
shutil.rmtree(_previous_results_dir, ignore_errors=True)
|
| 62 |
+
logger.debug("Cleaned up previous results: %s", _previous_results_dir)
|
| 63 |
+
|
| 64 |
logger.info("Running segmentation for %s", case_id)
|
| 65 |
result = run_pipeline_on_case(
|
| 66 |
case_id,
|
|
|
|
| 69 |
cleanup_staging=True,
|
| 70 |
)
|
| 71 |
|
| 72 |
+
# Track results_dir for cleanup on next run
|
| 73 |
+
_previous_results_dir = result.results_dir
|
| 74 |
+
|
| 75 |
# 1. NiiVue Visualization
|
| 76 |
# We need data URLs for the browser
|
| 77 |
# Note: This reads the file content into memory (base64)
|
|
@@ -26,7 +26,7 @@ class TestCli:
|
|
| 26 |
result = PipelineResult(
|
| 27 |
case_id="sub-001",
|
| 28 |
input_files=MagicMock(),
|
| 29 |
-
|
| 30 |
prediction_mask=MagicMock(),
|
| 31 |
ground_truth=None,
|
| 32 |
dice_score=None,
|
|
@@ -50,7 +50,7 @@ class TestCli:
|
|
| 50 |
result = PipelineResult(
|
| 51 |
case_id="sub-001",
|
| 52 |
input_files=MagicMock(),
|
| 53 |
-
|
| 54 |
prediction_mask=MagicMock(),
|
| 55 |
ground_truth=None,
|
| 56 |
dice_score=None,
|
|
|
|
| 26 |
result = PipelineResult(
|
| 27 |
case_id="sub-001",
|
| 28 |
input_files=MagicMock(),
|
| 29 |
+
results_dir=MagicMock(),
|
| 30 |
prediction_mask=MagicMock(),
|
| 31 |
ground_truth=None,
|
| 32 |
dice_score=None,
|
|
|
|
| 50 |
result = PipelineResult(
|
| 51 |
case_id="sub-001",
|
| 52 |
input_files=MagicMock(),
|
| 53 |
+
results_dir=MagicMock(),
|
| 54 |
prediction_mask=MagicMock(),
|
| 55 |
ground_truth=None,
|
| 56 |
dice_score=None,
|
|
@@ -35,20 +35,18 @@ class TestRunPipelineOnCase:
|
|
| 35 |
# Configure mocks
|
| 36 |
mock_dataset = MagicMock()
|
| 37 |
|
| 38 |
-
# Create real temp files
|
|
|
|
|
|
|
|
|
|
|
|
|
| 39 |
gt_file = temp_dir / "gt_mock.nii.gz"
|
| 40 |
-
gt_file.write_bytes(b"fake nifti")
|
| 41 |
-
|
| 42 |
-
# Mock paths that "exist"
|
| 43 |
-
dwi_path = MagicMock(spec=Path)
|
| 44 |
-
dwi_path.exists.return_value = True
|
| 45 |
-
adc_path = MagicMock(spec=Path)
|
| 46 |
-
adc_path.exists.return_value = True
|
| 47 |
|
| 48 |
mock_dataset.get_case.return_value = CaseFiles(
|
| 49 |
-
dwi=
|
| 50 |
-
adc=
|
| 51 |
-
ground_truth=gt_file,
|
| 52 |
# flair omitted
|
| 53 |
)
|
| 54 |
# Support context manager protocol: with load_isles_dataset() as dataset:
|
|
@@ -146,17 +144,18 @@ class TestRunPipelineOnCase:
|
|
| 146 |
def test_handles_missing_ground_truth(
|
| 147 |
self,
|
| 148 |
mock_dependencies: dict[str, MagicMock],
|
| 149 |
-
temp_dir: Path,
|
| 150 |
) -> None:
|
| 151 |
"""Handles cases without ground truth gracefully."""
|
| 152 |
-
#
|
| 153 |
-
|
| 154 |
-
|
| 155 |
-
|
| 156 |
-
|
|
|
|
| 157 |
mock_dependencies["dataset"].get_case.return_value = CaseFiles(
|
| 158 |
-
dwi=
|
| 159 |
-
adc=
|
| 160 |
# ground_truth omitted
|
| 161 |
)
|
| 162 |
|
|
@@ -237,7 +236,7 @@ class TestRunPipelineOnBatch:
|
|
| 237 |
PipelineResult(
|
| 238 |
case_id="sub-001",
|
| 239 |
input_files=MagicMock(),
|
| 240 |
-
|
| 241 |
prediction_mask=MagicMock(),
|
| 242 |
ground_truth=None,
|
| 243 |
dice_score=0.8,
|
|
@@ -246,7 +245,7 @@ class TestRunPipelineOnBatch:
|
|
| 246 |
PipelineResult(
|
| 247 |
case_id="sub-002",
|
| 248 |
input_files=MagicMock(),
|
| 249 |
-
|
| 250 |
prediction_mask=MagicMock(),
|
| 251 |
ground_truth=None,
|
| 252 |
dice_score=0.9,
|
|
@@ -267,7 +266,7 @@ class TestRunPipelineOnBatch:
|
|
| 267 |
mock_run.return_value = PipelineResult(
|
| 268 |
case_id="sub-001",
|
| 269 |
input_files=MagicMock(),
|
| 270 |
-
|
| 271 |
prediction_mask=MagicMock(),
|
| 272 |
ground_truth=None,
|
| 273 |
dice_score=0.8,
|
|
|
|
| 35 |
# Configure mocks
|
| 36 |
mock_dataset = MagicMock()
|
| 37 |
|
| 38 |
+
# Create real temp files (pipeline copies these to results_dir)
|
| 39 |
+
dwi_file = temp_dir / "dwi_mock.nii.gz"
|
| 40 |
+
dwi_file.write_bytes(b"fake dwi nifti")
|
| 41 |
+
adc_file = temp_dir / "adc_mock.nii.gz"
|
| 42 |
+
adc_file.write_bytes(b"fake adc nifti")
|
| 43 |
gt_file = temp_dir / "gt_mock.nii.gz"
|
| 44 |
+
gt_file.write_bytes(b"fake gt nifti")
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 45 |
|
| 46 |
mock_dataset.get_case.return_value = CaseFiles(
|
| 47 |
+
dwi=dwi_file,
|
| 48 |
+
adc=adc_file,
|
| 49 |
+
ground_truth=gt_file,
|
| 50 |
# flair omitted
|
| 51 |
)
|
| 52 |
# Support context manager protocol: with load_isles_dataset() as dataset:
|
|
|
|
| 144 |
def test_handles_missing_ground_truth(
|
| 145 |
self,
|
| 146 |
mock_dependencies: dict[str, MagicMock],
|
| 147 |
+
temp_dir: Path,
|
| 148 |
) -> None:
|
| 149 |
"""Handles cases without ground truth gracefully."""
|
| 150 |
+
# Create real files for DWI/ADC (pipeline copies these)
|
| 151 |
+
dwi_file = temp_dir / "dwi_no_gt.nii.gz"
|
| 152 |
+
dwi_file.write_bytes(b"fake dwi")
|
| 153 |
+
adc_file = temp_dir / "adc_no_gt.nii.gz"
|
| 154 |
+
adc_file.write_bytes(b"fake adc")
|
| 155 |
+
|
| 156 |
mock_dependencies["dataset"].get_case.return_value = CaseFiles(
|
| 157 |
+
dwi=dwi_file,
|
| 158 |
+
adc=adc_file,
|
| 159 |
# ground_truth omitted
|
| 160 |
)
|
| 161 |
|
|
|
|
| 236 |
PipelineResult(
|
| 237 |
case_id="sub-001",
|
| 238 |
input_files=MagicMock(),
|
| 239 |
+
results_dir=MagicMock(),
|
| 240 |
prediction_mask=MagicMock(),
|
| 241 |
ground_truth=None,
|
| 242 |
dice_score=0.8,
|
|
|
|
| 245 |
PipelineResult(
|
| 246 |
case_id="sub-002",
|
| 247 |
input_files=MagicMock(),
|
| 248 |
+
results_dir=MagicMock(),
|
| 249 |
prediction_mask=MagicMock(),
|
| 250 |
ground_truth=None,
|
| 251 |
dice_score=0.9,
|
|
|
|
| 266 |
mock_run.return_value = PipelineResult(
|
| 267 |
case_id="sub-001",
|
| 268 |
input_files=MagicMock(),
|
| 269 |
+
results_dir=MagicMock(),
|
| 270 |
prediction_mask=MagicMock(),
|
| 271 |
ground_truth=None,
|
| 272 |
dice_score=0.8,
|
|
@@ -4,8 +4,13 @@ from unittest.mock import MagicMock, patch
|
|
| 4 |
from stroke_deepisles_demo.pipeline import run_pipeline_on_case
|
| 5 |
|
| 6 |
|
| 7 |
-
def test_pipeline_cleanup_default() -> None:
|
| 8 |
"""Test that pipeline cleans up staging directory by default."""
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 9 |
|
| 10 |
# Mock everything to avoid running actual heavy inference
|
| 11 |
with (
|
|
@@ -13,13 +18,13 @@ def test_pipeline_cleanup_default() -> None:
|
|
| 13 |
patch("stroke_deepisles_demo.pipeline.stage_case_for_deepisles") as mock_stage,
|
| 14 |
patch("stroke_deepisles_demo.pipeline.run_deepisles_on_folder") as mock_run,
|
| 15 |
patch("stroke_deepisles_demo.pipeline.metrics.compute_dice"),
|
| 16 |
-
patch("shutil.rmtree") as mock_rmtree,
|
| 17 |
):
|
| 18 |
# Setup mocks
|
| 19 |
mock_dataset = MagicMock()
|
| 20 |
mock_dataset.list_case_ids.return_value = ["case1"]
|
| 21 |
-
# Return dict
|
| 22 |
-
mock_dataset.get_case.return_value = {"dwi":
|
| 23 |
|
| 24 |
# Support context manager protocol: with load_isles_dataset() as dataset:
|
| 25 |
mock_load.return_value.__enter__ = MagicMock(return_value=mock_dataset)
|
|
@@ -36,7 +41,7 @@ def test_pipeline_cleanup_default() -> None:
|
|
| 36 |
# Run pipeline with defaults (cleanup_staging=True is the default)
|
| 37 |
run_pipeline_on_case("case1")
|
| 38 |
|
| 39 |
-
# Verify that rmtree was called
|
| 40 |
assert mock_rmtree.called
|
| 41 |
|
| 42 |
# Get the path passed to stage_case_for_deepisles
|
|
|
|
| 4 |
from stroke_deepisles_demo.pipeline import run_pipeline_on_case
|
| 5 |
|
| 6 |
|
| 7 |
+
def test_pipeline_cleanup_default(temp_dir: Path) -> None:
|
| 8 |
"""Test that pipeline cleans up staging directory by default."""
|
| 9 |
+
# Create real files (pipeline now copies input files to results_dir)
|
| 10 |
+
dwi_file = temp_dir / "dwi.nii.gz"
|
| 11 |
+
dwi_file.write_bytes(b"fake dwi")
|
| 12 |
+
adc_file = temp_dir / "adc.nii.gz"
|
| 13 |
+
adc_file.write_bytes(b"fake adc")
|
| 14 |
|
| 15 |
# Mock everything to avoid running actual heavy inference
|
| 16 |
with (
|
|
|
|
| 18 |
patch("stroke_deepisles_demo.pipeline.stage_case_for_deepisles") as mock_stage,
|
| 19 |
patch("stroke_deepisles_demo.pipeline.run_deepisles_on_folder") as mock_run,
|
| 20 |
patch("stroke_deepisles_demo.pipeline.metrics.compute_dice"),
|
| 21 |
+
patch("stroke_deepisles_demo.pipeline.shutil.rmtree") as mock_rmtree,
|
| 22 |
):
|
| 23 |
# Setup mocks
|
| 24 |
mock_dataset = MagicMock()
|
| 25 |
mock_dataset.list_case_ids.return_value = ["case1"]
|
| 26 |
+
# Return dict with real files (no ground_truth)
|
| 27 |
+
mock_dataset.get_case.return_value = {"dwi": dwi_file, "adc": adc_file}
|
| 28 |
|
| 29 |
# Support context manager protocol: with load_isles_dataset() as dataset:
|
| 30 |
mock_load.return_value.__enter__ = MagicMock(return_value=mock_dataset)
|
|
|
|
| 41 |
# Run pipeline with defaults (cleanup_staging=True is the default)
|
| 42 |
run_pipeline_on_case("case1")
|
| 43 |
|
| 44 |
+
# Verify that rmtree was called (for staging cleanup)
|
| 45 |
assert mock_rmtree.called
|
| 46 |
|
| 47 |
# Get the path passed to stage_case_for_deepisles
|
|
@@ -51,7 +51,7 @@ def test_run_segmentation_logic() -> None:
|
|
| 51 |
mock_result = PipelineResult(
|
| 52 |
case_id="sub-001",
|
| 53 |
input_files={"dwi": MagicMock(), "adc": MagicMock()},
|
| 54 |
-
|
| 55 |
prediction_mask=MagicMock(),
|
| 56 |
ground_truth=MagicMock(),
|
| 57 |
dice_score=0.85,
|
|
|
|
| 51 |
mock_result = PipelineResult(
|
| 52 |
case_id="sub-001",
|
| 53 |
input_files={"dwi": MagicMock(), "adc": MagicMock()},
|
| 54 |
+
results_dir=MagicMock(),
|
| 55 |
prediction_mask=MagicMock(),
|
| 56 |
ground_truth=MagicMock(),
|
| 57 |
dice_score=0.85,
|