[wayfarer3130]

Adds ability to generate thumbnails for selected queries from DICOMweb or file system.

[jbocce]

This test regex seems to be done a lot. Is it worth centralizing somewhere?

[jbocce]

Here are some requests from Claude. I think the first one is the most important to address.

• HttpDicomWebReader._resolveBulkDataPath ignores instanceUID for instance-relative ./frames URIs (HttpDicomWebReader.mjs:55-65). The file reader distinguishes ./instances//frames (series-relative) from ./frames (instance-relative, needs instanceUID); the HTTP reader treats every ./-prefixed URI as series-relative. A server emitting BulkDataURI: "./frames" will resolve
  to studies//series//frames/ instead of studies//series//instances//frames/ — 404 or wrong frame. Match the file reader's rule.
• Redundant double upload in studiesMain.mjs:31-34: uploadDeploy is awaited twice in a row. This appears pre-existing (also in master) and is not fixed by this PR — worth fixing while you're in the file.
• thumbnailMain.mjs:831 HTTP-output guard fires after the more specific check: (outputDicomdir || dicomdir).startsWith('http') rejects an HTTP outputDicomdir, but the message "must be a file path" is misleading since s3:// is also accepted. Reword to "must be a local path or s3:// URI" and place it before the http-input check for clarity.
• No Accept headers on QIDO/WADO-RS fetches (HttpDicomWebReader._fetch). DICOMweb-strict servers may return 406 or wrong representations. Set Accept: application/dicom+json for QIDO paths and multipart/related; type="application/octet-stream" (or the appropriate transfer-syntax variant) for bulk frames.
• HTTP path has no retry / timeout / concurrency control. --all-thumbnails against a remote DICOMweb peer issues serial fetches with no backoff — slow on success, blocking on a single hang. Consider an AbortSignal with AbortSignal.timeout(...) per request, and Promise.all/p-limit for instance-level work.

[wayfarer3130]

@rleisti - I hvae this change to allow uploading the thumbnails in bulk to S3 or creating or re-creating them locally. It was for some OHIF test work, but I don't have a reviewer on the OHIF side who is familiar with this. Could you review the changes? I'd like to address the resolve bulkdata path as a separate issue, since the changes that were being directly suggested don't quite work without doing things in a different way.
I'm going to eventually follow the path of how OHIF does things, which is to add hidden data to bulkdata containing objects that have absolute paths plus the fetch function, but that is a bigger change than I'd like to include here.

[rleisti]

Added some initial comments: I have not gone through the entire review yet.

[rleisti]

In studyMainSingle, options.include is set to ['thumbnail']. If collectFiles is called for the directory studies/<UID> then it seems this code would skip any sub-directories that not include the substring 'thumbnail' (like 'series'), cause it to skip series thumbnails.

[rleisti]

commonMain is being called twice with identical arguments: is this intended?

[rleisti]

Reviewed at 1ad4b313e, merged locally with current master (1a38951).

The three comments from my first round are fixed. Directories are recursed again under --thumbnail-only, so series thumbnails are included. The duplicate commonMain upload is gone. The redundant --study-selector option was removed.

Blocking

1. Secondary-capture (Modality OT) images no longer get thumbnails, and the webserver now returns 404 for them

1. Take a single-frame MR image and set Modality to OT, which is the usual case for a Secondary Capture. Run createdicomweb create on it.
2. Run createdicomweb thumbnail <study> --dicomdir <dir>.
3. Observed: every level (instance, series, study) is skipped with Modality OT is omitted from thumbnail generation, and no file is written. On master, the same study gets a 256×256 thumbnail.
4. Run dicomwebserver on the same tree and GET .../series/<S>/thumbnail and .../instances/<I>/thumbnail. Observed: 404 on this branch, 200 image/jpeg on master. A CT series in the same tree returns 200 on both builds.
5. Expected: OT objects with pixel data render the same way they did before.

THUMBNAIL_OMIT_MODALITIES in thumbnailMain.mjs:54 lists OT, and thumbnailInstancePrecheck skips an instance on modality alone. The on-demand path in createMissingThumbnail can't pass --force, so the webserver has no way around the skip. Anyone browsing a study that contains secondary captures (screenshots, scanned images, exported reports) loses those thumbnails. Wanted: decide whether there is pixel data, not what the modality is. If a modality list stays, OT can't be on it.

2. The branch conflicts with master, and the index.mjs conflict has a wrong resolution that breaks Bun

GitHub reports the branch as dirty. A local merge of master conflicts in two files:

• create-dicomweb/bin/createdicomweb.mjs: formatting, plus master's new --max-open-files option, which the resolution has to keep.
• static-wado-creator/lib/index.mjs: master changed the uids import to import uids from './uids.js' (#133/#134). Keeping this branch's import { uids } from '@radicalimaging/static-wado-util' brings back the Bun require() async module failure and the jest _require failure those PRs fixed.

Resolved with master's import plus this PR's videoWriterFactory import, the full build and every package test suite pass.

Should fix

3. In the default mode, --frame-numbers is silently ignored

1. Run createdicomweb thumbnail <CT study> --frame-numbers 1-2 with neither --study-thumbnail nor --series-thumbnail.
2. Observed: 180 files, all named thumbnail, with no thumbnail-1. The same command with --series-thumbnail writes thumbnail-1.
3. Expected: the option does what its help text says in every mode, or the CLI rejects it where it doesn't apply.

The default loop in thumbnailMain.mjs:946 always renders Math.ceil(numberOfFrames / 2). Representative mode and the webserver's on-demand path use framesToProcess, which defaults to frame 1. So, reasoned from the code and not run because the test data has no multi-frame image: for a multi-frame instance, instances/<sop>/thumbnail is the middle frame or the first frame depending on which mode wrote it first. Without --force, the existing file is then kept.

4. No regression tests cover the new behaviour

The only new test is static-wado-util/test/unit/bulkDataUriResolve.jest.js. Untested:

• the three thumbnail modes and the modality/PhotometricInterpretation precheck: create-dicomweb/test, next to seriesSummaryOlderLayout.jest.mjs
• HttpDicomWebReader (QIDO mapping, multipart frame extraction)
• the DeployGroup include filter: extend static-wado-deploy/test/unit/DeployGroup.jest.mjs
• on-demand createMissingThumbnail: static-wado-webserver/test/unit

A test of the OT case would have caught 1.

Nit

5. createdicomweb thumbnail with no argument now runs on every study

The study UID used to be required. Now a missing argument falls back to 'true' (createdicomweb.mjs:326), so a forgotten argument against an http(s) dicomdir walks the whole remote archive. Consider requiring an explicit true. On the same line, options.studySelector no longer exists since the option was removed.

Not verified: S3 output (s3:// as --output-dicomdir) and deploy --thumbnail-only, because I had no bucket. The HTTP reader against a DICOMweb server other than dicomwebserver. Video and multi-frame thumbnails, because the test data has none. The HTTP reader against this repo's own webserver works: 176 instance, 3 series and 1 study thumbnail written for the Juno CT.