From 0625047a6999a4170a33be8f77cc0fb79149f332 Mon Sep 17 00:00:00 2001 From: Steve Laing Date: Thu, 19 Mar 2026 16:49:47 +0000 Subject: [PATCH 1/2] Add the return key attributes as specified in the conformance doc The documentation says non-requested keys will be ignored, so we are safe to always return the full set for each item. --- src/services/mwl/c_find.py | 41 +++++++++++++++++------ src/services/storage.py | 31 ++++++++++++++++++ tests/services/mwl/test_c_find.py | 54 ++++++++++++++++++++++--------- 3 files changed, 100 insertions(+), 26 deletions(-) diff --git a/src/services/mwl/c_find.py b/src/services/mwl/c_find.py index 20c19ed1..47964d75 100644 --- a/src/services/mwl/c_find.py +++ b/src/services/mwl/c_find.py @@ -88,26 +88,47 @@ def _build_worklist_response(self, item: WorklistItem) -> Dataset: ds.PatientID = item.patient_id ds.PatientName = item.patient_name ds.PatientBirthDate = item.patient_birth_date - if item.patient_sex: - ds.PatientSex = item.patient_sex + ds.PatientSex = item.patient_sex + ds.PatientAge = item.patient_age + ds.PatientWeight = item.patient_weight + ds.PatientAddress = item.patient_address + ds.PatientComments = item.patient_comments # Study information ds.AccessionNumber = item.accession_number - if item.study_instance_uid: - ds.StudyInstanceUID = item.study_instance_uid + ds.StudyInstanceUID = item.study_instance_uid - if item.study_description: - ds.StudyDescription = item.study_description - sps_item.ScheduledProcedureStepDescription = ds.StudyDescription + ds.StudyDescription = item.study_description + sps_item.ScheduledProcedureStepDescription = ds.StudyDescription - if item.procedure_code: - ds.RequestedProcedureID = item.procedure_code - sps_item.ScheduledProcedureStepID = ds.RequestedProcedureID + ds.RequestedProcedureID = item.procedure_code + + ds.RequestedProcedureCodeSequence = [Dataset()] + ds.RequestedProcedureCodeSequence[0].CodeValue = item.procedure_code + ds.RequestedProcedureCodeSequence[0].CodingSchemeDesignator = item.procedure_coding_scheme_designator + ds.RequestedProcedureCodeSequence[0].CodeMeaning = item.procedure_code_meaning + + ds.ReasonForRequestedProcedureCodeSequence = [Dataset()] + ds.ReasonForRequestedProcedureCodeSequence[0].CodeValue = item.reason_code_value + ds.ReasonForRequestedProcedureCodeSequence[0].CodingSchemeDesignator = item.reason_coding_scheme_designator + ds.ReasonForRequestedProcedureCodeSequence[0].CodeMeaning = item.reason_code_meaning + + sps_item.ScheduledProcedureStepID = ds.RequestedProcedureID # Scheduled Procedure Step Sequence sps_item.ScheduledProcedureStepStartDate = item.scheduled_date sps_item.ScheduledProcedureStepStartTime = item.scheduled_time sps_item.Modality = item.modality + sps_item.ScheduledStationAETitle = item.scheduled_station_aet + sps_item.ScheduledPerformingPhysicianName = item.scheduled_performing_physician_name + sps_item.ScheduledStationName = item.scheduled_station_name + sps_item.ScheduledProcedureStepLocation = item.scheduled_procedure_step_location + sps_item.ScheduledProtocolCodeSequence = [Dataset()] + sps_item.ScheduledProtocolCodeSequence[0].CodeValue = item.scheduled_protocol_code_value + sps_item.ScheduledProtocolCodeSequence[ + 0 + ].CodingSchemeDesignator = item.scheduled_protocol_coding_scheme_designator + sps_item.ScheduledProtocolCodeSequence[0].CodeMeaning = item.scheduled_protocol_code_meaning ds.ScheduledProcedureStepSequence = [sps_item] diff --git a/src/services/storage.py b/src/services/storage.py index 3028b7ed..4594d256 100644 --- a/src/services/storage.py +++ b/src/services/storage.py @@ -291,6 +291,7 @@ class WorklistItem: scheduled_date: str = field(doc="Date the procedure is scheduled for.") scheduled_time: str = field(doc="Time the procedure is scheduled for.") status: str = field(doc="Status of the worklist item", default=MWLStatus.SCHEDULED.value) + source_message_id: Optional[str] = field( default=None, doc="Message ID from system which created this worklist item", hash=True ) @@ -302,6 +303,36 @@ class WorklistItem: default=None, doc="Modality Performed Procedure Step (MPPS) instance UID if available." ) + patient_age: Optional[str] = field(default=None, doc="Age of the patient at the time of scheduling.") + patient_weight: Optional[str] = field(default=None, doc="Weight of the patient at the time of scheduling.") + patient_address: Optional[str] = field(default=None, doc="Address of the patient.") + patient_comments: Optional[str] = field(default=None, doc="Additional comments about the patient.") + + procedure_coding_scheme_designator: Optional[str] = field( + default=None, doc="Coding scheme designator for the procedure code." + ) + procedure_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the procedure code.") + + reason_code_value: Optional[str] = field(default=None, doc="Code value for the reason for requested procedure.") + reason_coding_scheme_designator: Optional[str] = field( + default=None, doc="Coding scheme designator for the reason for requested procedure." + ) + reason_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the reason for requested procedure.") + + scheduled_performing_physician_name: Optional[str] = field( + default=None, doc="Name of the scheduled performing physician." + ) + scheduled_procedure_step_location: Optional[str] = field( + default=None, doc="Location of the scheduled procedure step." + ) + scheduled_station_aet: Optional[str] = field(default=None, doc="AE Title of the scheduled station.") + scheduled_station_name: Optional[str] = field(default=None, doc="Name of the scheduled station.") + scheduled_protocol_code_value: Optional[str] = field(default=None, doc="Code value for the scheduled protocol.") + scheduled_protocol_coding_scheme_designator: Optional[str] = field( + default=None, doc="Coding scheme designator for the scheduled protocol." + ) + scheduled_protocol_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the scheduled protocol.") + class WorklistItemNotFoundError(Exception): """Raised when a worklist item is not found in storage.""" diff --git a/tests/services/mwl/test_c_find.py b/tests/services/mwl/test_c_find.py index 309ee74f..5df524fa 100644 --- a/tests/services/mwl/test_c_find.py +++ b/tests/services/mwl/test_c_find.py @@ -215,23 +215,45 @@ def test_call_handles_storage_exception(self, handler, mock_storage, mock_event) assert status == FAILURE assert ds is None - def test_build_worklist_response_missing_optional_fields(self, handler): - minimal_item = WorklistItem( - accession_number="ACC001", - patient_id="9876543210", - patient_name="TEST^PATIENT", - patient_birth_date="19800101", - scheduled_date="20260107", - scheduled_time="100000", - modality="MG", - ) + def test_call_return_key_attributes_present(self, handler, mock_storage, mock_event, sample_worklist_item): + worklist_item = WorklistItem(**sample_worklist_item) + mock_storage.find_worklist_items.return_value = [worklist_item] - ds = handler._build_worklist_response(minimal_item) + results = list(handler.call(mock_event)) - # Required fields present + assert len(results) == 2 + status, ds = results[0] + assert status == PENDING assert ds.PatientID == "9876543210" assert ds.AccessionNumber == "ACC001" - # Optional fields absent - assert not hasattr(ds, "PatientSex") - assert not hasattr(ds, "StudyInstanceUID") - assert not hasattr(ds, "StudyDescription") + assert ds.PatientName == "TEST^PATIENT" + assert ds.PatientBirthDate == "19800101" + + assert ds.PatientAddress is None + assert ds.PatientComments is None + assert ds.PatientWeight is None + assert ds.PatientAge is None + assert ds.PatientSex == "F" + + assert ds.StudyDescription == "Bilateral Screening Mammogram" + assert ds.StudyInstanceUID == "1.2.3.4.5" # gitleaks:allow + + scheduled_procedure_step = ds.ScheduledProcedureStepSequence[0] + + assert scheduled_procedure_step.Modality == "MG" + assert scheduled_procedure_step.ScheduledProcedureStepStartDate == "20260107" + assert scheduled_procedure_step.ScheduledProcedureStepStartTime == "100000" + assert scheduled_procedure_step.ScheduledProcedureStepID == "PROC001" + assert scheduled_procedure_step.ScheduledStationAETitle is None + assert scheduled_procedure_step.ScheduledStationName is None + assert scheduled_procedure_step.ScheduledProtocolCodeSequence[0].CodeValue is None + assert scheduled_procedure_step.ScheduledProtocolCodeSequence[0].CodingSchemeDesignator is None + assert scheduled_procedure_step.ScheduledProtocolCodeSequence[0].CodeMeaning is None + + assert ds.ReasonForRequestedProcedureCodeSequence[0].CodeValue is None + assert ds.ReasonForRequestedProcedureCodeSequence[0].CodingSchemeDesignator is None + assert ds.ReasonForRequestedProcedureCodeSequence[0].CodeMeaning is None + + assert ds.RequestedProcedureCodeSequence[0].CodeValue == "PROC001" + assert ds.RequestedProcedureCodeSequence[0].CodingSchemeDesignator is None + assert ds.RequestedProcedureCodeSequence[0].CodeMeaning is None From ded5bc763cf673fbfb0077175df2a3a35a648e10 Mon Sep 17 00:00:00 2001 From: Steve Laing Date: Mon, 23 Mar 2026 10:48:54 +0000 Subject: [PATCH 2/2] Move WorklistItem into models.py The storage service is quite large and with additional return key fields the WorklistItem dataclass was adding bloat. We all understand what models.py does. --- src/models.py | 59 ++++++++++++++++++++++++ src/services/mwl/c_find.py | 3 +- src/services/mwl/create_worklist_item.py | 3 +- src/services/storage.py | 58 +---------------------- tests/services/test_storage.py | 3 +- tests/test_relay_listener.py | 2 +- 6 files changed, 67 insertions(+), 61 deletions(-) create mode 100644 src/models.py diff --git a/src/models.py b/src/models.py new file mode 100644 index 00000000..17945e85 --- /dev/null +++ b/src/models.py @@ -0,0 +1,59 @@ +from dataclasses import dataclass, field +from typing import Optional + +from services.mwl import MWLStatus + + +@dataclass +class WorklistItem: + accession_number: str = field( + doc="A departmental Information System generated number that identifies the Imaging Service Request.", + ) + modality: str = field(doc="Code for type of equipment that will perform the procedure.") + patient_birth_date: str = field(doc="Date of Birth of the Patient.") + patient_id: str = field(doc="Patient NHS Number", hash=True) + patient_name: str = field(doc="Name of the patient. Lastname^Firstname.") + scheduled_date: str = field(doc="Date the procedure is scheduled for.") + scheduled_time: str = field(doc="Time the procedure is scheduled for.") + status: str = field(doc="Status of the worklist item", default=MWLStatus.SCHEDULED.value) + + source_message_id: Optional[str] = field( + default=None, doc="Message ID from system which created this worklist item", hash=True + ) + study_instance_uid: Optional[str] = field(default=None, doc="Instance UID for the study", hash=True) + procedure_code: Optional[str] = field(default=None, doc="Code that identifies the requested procedure.") + patient_sex: Optional[str] = field(default=None, doc="Sex of the patient.") + study_description: Optional[str] = field(default=None, doc="Description of the study.") + mpps_instance_uid: Optional[str] = field( + default=None, doc="Modality Performed Procedure Step (MPPS) instance UID if available." + ) + + patient_age: Optional[str] = field(default=None, doc="Age of the patient at the time of scheduling.") + patient_weight: Optional[str] = field(default=None, doc="Weight of the patient at the time of scheduling.") + patient_address: Optional[str] = field(default=None, doc="Address of the patient.") + patient_comments: Optional[str] = field(default=None, doc="Additional comments about the patient.") + + procedure_coding_scheme_designator: Optional[str] = field( + default=None, doc="Coding scheme designator for the procedure code." + ) + procedure_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the procedure code.") + + reason_code_value: Optional[str] = field(default=None, doc="Code value for the reason for requested procedure.") + reason_coding_scheme_designator: Optional[str] = field( + default=None, doc="Coding scheme designator for the reason for requested procedure." + ) + reason_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the reason for requested procedure.") + + scheduled_performing_physician_name: Optional[str] = field( + default=None, doc="Name of the scheduled performing physician." + ) + scheduled_procedure_step_location: Optional[str] = field( + default=None, doc="Location of the scheduled procedure step." + ) + scheduled_station_aet: Optional[str] = field(default=None, doc="AE Title of the scheduled station.") + scheduled_station_name: Optional[str] = field(default=None, doc="Name of the scheduled station.") + scheduled_protocol_code_value: Optional[str] = field(default=None, doc="Code value for the scheduled protocol.") + scheduled_protocol_coding_scheme_designator: Optional[str] = field( + default=None, doc="Coding scheme designator for the scheduled protocol." + ) + scheduled_protocol_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the scheduled protocol.") diff --git a/src/services/mwl/c_find.py b/src/services/mwl/c_find.py index 47964d75..61e495af 100644 --- a/src/services/mwl/c_find.py +++ b/src/services/mwl/c_find.py @@ -10,8 +10,9 @@ from pydicom import Dataset from pynetdicom import evt +from models import WorklistItem from services.dicom import CHARSET_UTF8, FAILURE, PENDING, SUCCESS -from services.storage import MWLStorage, WorklistItem +from services.storage import MWLStorage logger = logging.getLogger(__name__) diff --git a/src/services/mwl/create_worklist_item.py b/src/services/mwl/create_worklist_item.py index b743523e..6dd3fd77 100644 --- a/src/services/mwl/create_worklist_item.py +++ b/src/services/mwl/create_worklist_item.py @@ -1,6 +1,7 @@ import logging -from services.storage import DuplicateWorklistItemError, MWLStorage, WorklistItem +from models import WorklistItem +from services.storage import DuplicateWorklistItemError, MWLStorage logger = logging.getLogger(__name__) diff --git a/src/services/storage.py b/src/services/storage.py index 4594d256..3b55356e 100644 --- a/src/services/storage.py +++ b/src/services/storage.py @@ -3,11 +3,10 @@ import os import sqlite3 from contextlib import contextmanager -from dataclasses import dataclass, field from pathlib import Path from typing import Dict, List, Optional -from services.mwl import MWLStatus +from models import WorklistItem logger = logging.getLogger(__name__) @@ -279,61 +278,6 @@ def mark_upload_failed(self, sop_instance_uid: str, error: str, permanent: bool conn.commit() -@dataclass -class WorklistItem: - accession_number: str = field( - doc="A departmental Information System generated number that identifies the Imaging Service Request.", - ) - modality: str = field(doc="Code for type of equipment that will perform the procedure.") - patient_birth_date: str = field(doc="Date of Birth of the Patient.") - patient_id: str = field(doc="Patient NHS Number", hash=True) - patient_name: str = field(doc="Name of the patient. Lastname^Firstname.") - scheduled_date: str = field(doc="Date the procedure is scheduled for.") - scheduled_time: str = field(doc="Time the procedure is scheduled for.") - status: str = field(doc="Status of the worklist item", default=MWLStatus.SCHEDULED.value) - - source_message_id: Optional[str] = field( - default=None, doc="Message ID from system which created this worklist item", hash=True - ) - study_instance_uid: Optional[str] = field(default=None, doc="Instance UID for the study", hash=True) - procedure_code: Optional[str] = field(default=None, doc="Code that identifies the requested procedure.") - patient_sex: Optional[str] = field(default=None, doc="Sex of the patient.") - study_description: Optional[str] = field(default=None, doc="Description of the study.") - mpps_instance_uid: Optional[str] = field( - default=None, doc="Modality Performed Procedure Step (MPPS) instance UID if available." - ) - - patient_age: Optional[str] = field(default=None, doc="Age of the patient at the time of scheduling.") - patient_weight: Optional[str] = field(default=None, doc="Weight of the patient at the time of scheduling.") - patient_address: Optional[str] = field(default=None, doc="Address of the patient.") - patient_comments: Optional[str] = field(default=None, doc="Additional comments about the patient.") - - procedure_coding_scheme_designator: Optional[str] = field( - default=None, doc="Coding scheme designator for the procedure code." - ) - procedure_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the procedure code.") - - reason_code_value: Optional[str] = field(default=None, doc="Code value for the reason for requested procedure.") - reason_coding_scheme_designator: Optional[str] = field( - default=None, doc="Coding scheme designator for the reason for requested procedure." - ) - reason_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the reason for requested procedure.") - - scheduled_performing_physician_name: Optional[str] = field( - default=None, doc="Name of the scheduled performing physician." - ) - scheduled_procedure_step_location: Optional[str] = field( - default=None, doc="Location of the scheduled procedure step." - ) - scheduled_station_aet: Optional[str] = field(default=None, doc="AE Title of the scheduled station.") - scheduled_station_name: Optional[str] = field(default=None, doc="Name of the scheduled station.") - scheduled_protocol_code_value: Optional[str] = field(default=None, doc="Code value for the scheduled protocol.") - scheduled_protocol_coding_scheme_designator: Optional[str] = field( - default=None, doc="Coding scheme designator for the scheduled protocol." - ) - scheduled_protocol_code_meaning: Optional[str] = field(default=None, doc="Code meaning for the scheduled protocol.") - - class WorklistItemNotFoundError(Exception): """Raised when a worklist item is not found in storage.""" diff --git a/tests/services/test_storage.py b/tests/services/test_storage.py index 07474e3e..73b0a27d 100644 --- a/tests/services/test_storage.py +++ b/tests/services/test_storage.py @@ -5,7 +5,8 @@ import pytest from pydicom.uid import generate_uid -from services.storage import MWLStorage, PACSStorage, WorklistItem, WorklistItemNotFoundError +from models import WorklistItem +from services.storage import MWLStorage, PACSStorage, WorklistItemNotFoundError @patch("services.storage.sqlite3") diff --git a/tests/test_relay_listener.py b/tests/test_relay_listener.py index 38917cd7..a3595790 100644 --- a/tests/test_relay_listener.py +++ b/tests/test_relay_listener.py @@ -5,8 +5,8 @@ from websockets.exceptions import ConnectionClosedError from websockets.frames import Close, CloseCode +from models import WorklistItem from relay_listener import RelayListener, RelayURI, main -from services.storage import WorklistItem class TestRelayListener: