From d11a5270880daa030558e3cead74d8b59efb9af7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 14:55:04 +0000 Subject: [PATCH] =?UTF-8?q?fix(pipeline):=20key=20per-series=20results=20u?= =?UTF-8?q?niquely=20=E2=80=94=20duplicate=20descriptions=20no=20longer=20?= =?UTF-8?q?drop=20series?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Volumes, detected planes, and everything keyed off context.volumes downstream (anomaly scores, top slices, vision analysis, report sections) used series.description as dict key. SeriesDescription is an optional DICOM tag that is frequently empty or repeated, so colliding series silently overwrote each other and studies lost entire series with no warning. Keys are now generated once per study: unique descriptions stay verbatim (labels unchanged for the common case), empty ones fall back to the series number, duplicates get a numeric suffix. Fixes #135 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4svt5QTs4WUMSy4HwiVt9 --- src/medcheck/pipeline/preprocess.py | 40 +++++++++++++++--- tests/unit/test_pipeline/test_preprocess.py | 45 +++++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/medcheck/pipeline/preprocess.py b/src/medcheck/pipeline/preprocess.py index 7a00ebb..834f384 100644 --- a/src/medcheck/pipeline/preprocess.py +++ b/src/medcheck/pipeline/preprocess.py @@ -134,19 +134,46 @@ def _build_volume(series: DicomSeries) -> np.ndarray: return volume +def _series_keys(series_list: list[DicomSeries]) -> list[str]: + """Return one unique, human-readable key per series. + + SeriesDescription (0008,103E) is optional and frequently empty or repeated + across series, so it cannot be used as a dict key directly — colliding + series would silently overwrite each other downstream (volumes, anomaly + scores, report sections). Unique descriptions are kept verbatim; empty + ones fall back to the series number, and duplicates get a numeric suffix. + """ + keys: list[str] = [] + used: set[str] = set() + for index, series in enumerate(series_list): + base = series.description or f"series-{series.series_number or index + 1}" + key = base + suffix = 2 + while key in used: + key = f"{base} ({suffix})" + suffix += 1 + used.add(key) + keys.append(key) + return keys + + class PreprocessStep(PipelineStep): """Build normalised numpy volumes from raw DICOM series.""" name: str = "preprocess" def run(self, context: PipelineContext) -> PipelineContext: - for series in context.dicom_series: + # All downstream per-series dicts (volumes, detected_planes, and the + # ml/vision results keyed off context.volumes) share these keys. + keys = _series_keys(context.dicom_series) + + for key, series in zip(keys, context.dicom_series, strict=True): try: - context.volumes[series.description] = _build_volume(series) + context.volumes[key] = _build_volume(series) except Exception as exc: # One malformed series must not abort the whole study; surface # the gap in the report's limitations instead. - message = f"Series '{series.description}' skipped during preprocessing: {exc}" + message = f"Series '{key}' skipped during preprocessing: {exc}" console.print(f"[yellow]{message}[/yellow]") context.limitations.append(message) @@ -155,8 +182,9 @@ def run(self, context: PipelineContext) -> PipelineContext: first_desc = context.dicom_series[0].description context.detected_anatomy = detect_anatomy(first_desc) - # Detect plane for every series - for series in context.dicom_series: - context.detected_planes[series.description] = detect_plane(series.description) + # Detect plane for every series (from the real description, keyed by + # the same unique key as the volume) + for key, series in zip(keys, context.dicom_series, strict=True): + context.detected_planes[key] = detect_plane(series.description) return context diff --git a/tests/unit/test_pipeline/test_preprocess.py b/tests/unit/test_pipeline/test_preprocess.py index 63a170b..0dcb034 100644 --- a/tests/unit/test_pipeline/test_preprocess.py +++ b/tests/unit/test_pipeline/test_preprocess.py @@ -137,3 +137,48 @@ def test_preprocess_bad_series_does_not_abort_study(): assert "broken" not in result.volumes assert "good" in result.volumes assert any("broken" in note for note in result.limitations) + + +def test_preprocess_duplicate_descriptions_keep_all_series(): + s1 = DicomSeries( + description="pd_sag", + series_number=1, + modality="MR", + slices=[_make_slice(instance_num=i, slice_loc=float(i)) for i in range(3)], + ) + s2 = DicomSeries( + description="pd_sag", # identical description — must not overwrite s1 + series_number=2, + modality="MR", + slices=[_make_slice(instance_num=i, slice_loc=float(i)) for i in range(4)], + ) + ctx = PipelineContext(dicom_series=[s1, s2]) + + result = PreprocessStep().run(ctx) + + assert len(result.volumes) == 2 + assert result.volumes["pd_sag"].shape[0] == 3 + assert result.volumes["pd_sag (2)"].shape[0] == 4 + assert set(result.detected_planes) == {"pd_sag", "pd_sag (2)"} + + +def test_preprocess_empty_descriptions_keep_all_series(): + s1 = DicomSeries( + description="", + series_number=4, + modality="MR", + slices=[_make_slice(instance_num=i, slice_loc=float(i)) for i in range(2)], + ) + s2 = DicomSeries( + description="", + series_number=0, # unset series number falls back to positional index + modality="MR", + slices=[_make_slice(instance_num=i, slice_loc=float(i)) for i in range(2)], + ) + ctx = PipelineContext(dicom_series=[s1, s2]) + + result = PreprocessStep().run(ctx) + + assert len(result.volumes) == 2 + assert "series-4" in result.volumes + assert "series-2" in result.volumes