Implement modality worklist by steventux · Pull Request #7 · NHSDigital/manage-breast-screening-gateway

steventux · 2026-01-06T12:41:51Z

Description

Adds Modality Worklist (MWL) server with C-FIND handler.
Supports C-FIND with patient ID, scheduled date and modality filters.
Provides a way to configure the MWL SQLite database separate from the PACS instance.
Allows Docker container to run both servers in separate threads.
Adds a script which can create worklist items from the CLI.

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11839

Review notes

Review checklist

Check database queries are correctly scoped to current_provider

Adds basic CRUD operations for worklist_items

Also fix return type of find_worklist_items

These differ slightly from Dict.

And an ADR for our threading policy

As opposed to separate threads

MatMoore

Have a few suggestions inline, but overall looks good. Would you mind squashing the ADR commits as the earlier one is redundant now?

It was a bit hard for me to spot whether we are diverging from spec at all... I'm not sure if there is somebody who is more familiar who can look at it from that angle, or if there is tooling we can use or are already using to check conformance?

MatMoore · 2026-01-12T09:31:02Z

src/services/init_worklist_db.sql

+
+    -- Scheduling information
+    scheduled_date TEXT NOT NULL,            -- YYYYMMDD format
+    scheduled_time TEXT NOT NULL,            -- HHMMSS format


does the protocol have a timezone to go with this time? If not it might be helpful to include in the comment what we expect it to be when stored in this db (e.g. UTC).

MatMoore · 2026-01-12T09:47:34Z

src/services/init_worklist_db.sql

+
+    -- DICOM identifiers
+    study_instance_uid TEXT,
+    mpps_instance_uid TEXT,


could we define the acronym MPPS in a comment?

MatMoore · 2026-01-12T09:52:55Z

src/services/init_worklist_db.sql

+CREATE INDEX IF NOT EXISTS idx_worklist_status
+ON worklist_items(status);
+
+-- Index for MPPS lookups by study instance UID


do we want an index on mpps_instance_uid as well?

MatMoore · 2026-01-12T10:20:02Z

src/services/storage.py

+        """
+        Add a new worklist item.
+
+        Args:


How about extracting this long list of args into a dataclass for the worklist item?

This method then becomes add_worklist_item(self, item: WorklistItem)

find_worklist_item could then return list[WorklistItem] rather than list[Dict], pywright will be able to fully type check it, and intellisense will be able to autocomplete the individual attributes of the return object.

It looks like since 3.14, if you use dataclass.field you can pass a doc attribute as well so you can document the fields individually https://docs.python.org/3/library/dataclasses.html#dataclasses.field

9159198 a lot cleaner than working with a dictionary 👍

MatMoore · 2026-01-12T10:24:52Z

src/services/storage.py

+                        patient_sex, scheduled_date, scheduled_time, modality,
+                        study_description, procedure_code, study_instance_uid,
+                        source_message_id
+                    ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)


sqlite3 and the python db api also support a named placeholder style, which is a bit less error prone when you have a lot of params.

Example from the docs:

# This is the named style used with executemany(): data = ( {"name": "C", "year": 1972}, {"name": "Fortran", "year": 1957}, {"name": "Python", "year": 1991}, {"name": "Go", "year": 2009}, ) cur.executemany("INSERT INTO lang VALUES(:name, :year)", data)

MatMoore · 2026-01-12T11:35:50Z

docs/adr/ADR-003_Multi_threaded_PACS_MWL_server.md

+- **Operational simplicity** - Single container to deploy, monitor, and manage
+- **Shared resources** - Both services share the same volume mounts and environment configuration
+- **pynetdicom compatibility** - The library's `AE.start_server()` is designed to block and handle connections, making threads a natural fit
+- **Lower overhead** - Threads have less overhead than separate processes or containers


What resources were we concerned about here?

MatMoore · 2026-01-12T11:39:34Z

docs/adr/ADR-003_Multi_threaded_PACS_MWL_server.md

+**Why this approach:**
+
+- **Operational simplicity** - Single container to deploy, monitor, and manage
+- **Shared resources** - Both services share the same volume mounts and environment configuration


Is this a circular argument? If we picked one of the other options I don't think this would be true?

Do we need to share the same volume mounts, and environment configuration? Or is this just the same argument of operational simplicity?

MatMoore · 2026-01-12T11:42:29Z

docs/adr/ADR-003_Multi_threaded_PACS_MWL_server.md

+
+- **No independent scaling or deployment** - Cannot scale or deploy PACS and MWL separately if one receives significantly more load
+- **Restart granularity** - Cannot restart one service without restarting both
+


Another issue is they share a logging configuration, and potentially configuration for other instrumentation. Maybe this can be mitigated by including the thread name in the format if you haven't done so already Never mind- this is addressed in the final ADR

docs/mwl/README.md

docs/adr/ADR-003_Separate_containers_for_PACS_and_MWL.md

Using a dataclass allows us to document and formalise the structure of data we store and retrieve from the database. "

steventux changed the title ~~WIP refactor storage to accommodate worklist db~~ Implement modality worklist Jan 6, 2026

Add WorklistStorage

19d6a04

Adds basic CRUD operations for worklist_items

steventux force-pushed the feat/modality-worklist branch from 9c237de to 19d6a04 Compare January 6, 2026 14:58

carlosmartinez force-pushed the feat/modality-worklist branch from 7bca5a3 to 83b3027 Compare January 6, 2026 16:05

steventux and others added 10 commits January 7, 2026 17:49

Ignore gitleaks false positives

37ec431

Add class MWLServer and corresponding test

258e20f

Fix indentation error

8bc758a

Add modality worklist C-FIND functionality

b63c101

Explicitly commit inserts and updates to worklist

7e8ca55

Also fix return type of find_worklist_items

Process query results as sqlite3.Row instances

5e841a7

These differ slightly from Dict.

Add integration tests for MWL C-FIND operations

2a39f6c

Update gateway entry point to include modality worklist

e194f56

Add Python script to add worklist items from cli

d9f9ef2

Improve docs in storage.py

ec431c8

steventux force-pushed the feat/modality-worklist branch from abbeb6a to ec431c8 Compare January 7, 2026 17:49

Rename WorklistStorage to MWLStorage for consistency

ee124f0

steventux marked this pull request as ready for review January 8, 2026 10:37

Documentation for MWL C-FIND functionality

74282aa

And an ADR for our threading policy

steventux requested a review from a team January 8, 2026 14:22

Run PACS and MWL in separate containers (#8)

931bd8a

As opposed to separate threads

steventux mentioned this pull request Jan 12, 2026

Implement gateway listener with add worklist item action handler #9

Merged

1 task

MatMoore approved these changes Jan 12, 2026

View reviewed changes

carlosmartinez and others added 8 commits January 12, 2026 14:04

Add more detail to decision record on using separate containers

8151180

MWL README: Add hyperlinks to DICOM documentation

7e7ebcf

Use WorklistItem dataclass for storage operations

9159198

Using a dataclass allows us to document and formalise the structure of data we store and retrieve from the database. "

Add index on MWL mpps_instance_id

02aab00

Document mpps_instance_uid field

2204d42

Document timezone used for MWL scheduled date/time

adb2a79

Return WorklistItem instance from get_worklist_item storage method

afbd7b9

Simplify C-FIND query parameter extraction

85d4c2a

steventux added 2 commits January 13, 2026 11:11

Raise when update/delete by accession number fails to find record

ee50746

Remove worklist item script, not currently used.

b538cb6

steventux merged commit 52bfa28 into main Jan 13, 2026
4 checks passed

steventux deleted the feat/modality-worklist branch January 13, 2026 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Implement modality worklist#7

Implement modality worklist#7
steventux merged 24 commits intomainfrom
feat/modality-worklist

steventux commented Jan 6, 2026 •

edited

Loading

Uh oh!

MatMoore left a comment •

edited

Loading

Uh oh!

MatMoore Jan 12, 2026

Uh oh!

MatMoore Jan 12, 2026

Uh oh!

MatMoore Jan 12, 2026

Uh oh!

MatMoore Jan 12, 2026

Uh oh!

steventux Jan 13, 2026

Uh oh!

MatMoore Jan 12, 2026

Uh oh!

MatMoore Jan 12, 2026 •

edited

Loading

Uh oh!

MatMoore Jan 12, 2026

Uh oh!

MatMoore Jan 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		- No independent scaling or deployment - Cannot scale or deploy PACS and MWL separately if one receives significantly more load
		- Restart granularity - Cannot restart one service without restarting both

Comments

Conversation

steventux commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Jira link

Review notes

Review checklist

Uh oh!

MatMoore left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

steventux Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

MatMoore Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

steventux commented Jan 6, 2026 •

edited

Loading

MatMoore left a comment •

edited

Loading

MatMoore Jan 12, 2026 •

edited

Loading

MatMoore Jan 12, 2026 •

edited

Loading