fix(pt): resolve Philips PET private SUV bulkdata before scaling (#6096)

* fix(pt): resolve Philips PET private SUV bulkdata before scaling

When a DICOMweb server delivers the Philips PET private tags SUVScaleFactor
(7053,1000) / ActivityConcentrationScaleFactor (7053,1009) as bulkdata, dcmjs
naturalization leaves them as { BulkDataURI } objects. These were fed verbatim
to calculate-suv, which treats the object as a valid value and silently
corrupts the SUV scaling factors.

Resolve these scalar private tags to numbers during ingestion - in both the
lazy (async) and non-lazy (sync) DICOMweb metadata paths, before INSTANCES_ADDED
fires - by decoding the bulkdata (VR-aware: DS/IS text or little-endian FL/FD).
Harden getPTImageIdInstanceMetadata to coerce values to finite numbers (reusing
@ohif/core utils.toNumber) and reject unresolved bulkdata objects so they can
never reach calculate-suv. Share the bulkdata-attach helper between both
metadata paths. Adds unit tests for the bulkdata decoder/resolver and for
getPTImageIdInstanceMetadata.

* refactor(bulkdata): move PET bulkdata resolution to a generic core tag registry

Generalize resolvePETPrivateScalarBulkData into a datasource-agnostic
utils.resolveBulkDataTags in @ohif/core, backed by a static tag registry
seeded with the Philips PET SUV/activity-concentration scalar tags and
extensible via registerResolvedBulkDataTags.

* fix(bulkdata): strip NUL padding and refresh qido auth before resolution

Address review feedback:
- decodeText now strips NUL (0x00) padding, which String.trim() leaves
  intact, so NUL-padded DS/IS values no longer decode as NaN. Adds a
  regression test.
- refresh qidoDicomWebClient.headers before resolveBulkDataTags in both
  series-metadata paths; retrieveBulkData is bound to qidoDicomWebClient,
  matching every other qido op in this file.

* fix(dicomweb): await deferred metadata storage

* fix(dicomweb): support single-part bulkdata responses

extensions/default/src/DicomWebDataSource/index.ts

@@ -25,6 +25,7 @@ import {
 } from '../utils/dicomWriter';
 import { getGetThumbnailSrc, ThumbnailContext } from './retrieveThumbnail';
 import { getRenderedURL } from './retrieveRendered';
+import retrieveBulkData from './retrieveBulkData';
 
 const { DicomMetaDictionary, DicomDict } = dcmjs.data;
 
@@ -147,6 +148,54 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) {
   // this is part of hte base standard.
   dicomWebConfig.bulkDataURI ||= { enabled: true };
 
+  /**
+   * Adds the retrieve bulkdata function to naturalized DICOM data.
+   * This is done recursively, for sub-sequences. Shared by both the lazy
+   * (async) and non-lazy (sync) series-metadata retrieval paths.
+   */
+  const addRetrieveBulkDataNaturalized = (naturalized, instance = naturalized) => {
+    if (!naturalized) {
+      return naturalized;
+    }
+    for (const key of Object.keys(naturalized)) {
+      const value = naturalized[key];
+
+      if (Array.isArray(value) && typeof value[0] === 'object') {
+        // Fix recursive values
+        const validValues = value.filter(Boolean);
+        validValues.forEach(child => addRetrieveBulkDataNaturalized(child, instance));
+        continue;
+      }
+
+      // The value.Value will be set with the bulkdata read value
+      // in which case it isn't necessary to re-read this.
+      if (value && value.BulkDataURI && !value.Value) {
+        // handle the scenarios where bulkDataURI is relative path
+        fixBulkDataURI(value, instance, dicomWebConfig);
+        // Provide a method to fetch bulkdata
+        value.retrieveBulkData = retrieveBulkData.bind(qidoDicomWebClient, value);
+      }
+    }
+    return naturalized;
+  };
+
+  /**
+   * naturalizes the dataset, and adds a retrieve bulkdata method
+   * to any values containing BulkDataURI.
+   * @param {*} instance
+   * @returns naturalized dataset, with retrieveBulkData methods
+   */
+  const addRetrieveBulkData = instance => {
+    const naturalized = naturalizeDataset(instance);
+
+    // if we know the server doesn't use bulkDataURI, then don't
+    if (!dicomWebConfig.bulkDataURI?.enabled) {
+      return naturalized;
+    }
+
+    return addRetrieveBulkDataNaturalized(naturalized);
+  };
+
   const implementation = {
     initialize: ({ params, query }) => {
       if (dicomWebConfig.onConfiguration && typeof dicomWebConfig.onConfiguration === 'function') {
@@ -501,8 +550,16 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) {
         dicomWebConfig
       );
 
-      // first naturalize the data
-      const naturalizedInstancesMetadata = data.map(naturalizeDataset);
+      // first naturalize the data, attaching bulkdata retrieve methods so that
+      // bulkdata-valued tags can be resolved (matching the lazy-load path).
+      const naturalizedInstancesMetadata = data.map(addRetrieveBulkData);
+
+      // Resolve the registered bulkdata tags (e.g. the Philips SUV Scale
+      // Factor) delivered as bulkdata into plain numbers BEFORE
+      // INSTANCES_ADDED fires. retrieveBulkData is bound to qidoDicomWebClient,
+      // so refresh its auth headers first (matching every other qido op here).
+      qidoDicomWebClient.headers = getAuthorizationHeader();
+      await utils.resolveBulkDataTags(naturalizedInstancesMetadata);
 
       const seriesSummaryMetadata = {};
       const instancesPerSeries = {};
@@ -576,57 +633,19 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) {
           dicomWebConfig
         );
 
-      /**
-       * Adds the retrieve bulkdata function to naturalized DICOM data.
-       * This is done recursively, for sub-sequences.
-       */
-      const addRetrieveBulkDataNaturalized = (naturalized, instance = naturalized) => {
-        if (!naturalized) {
-          return naturalized;
-        }
-        for (const key of Object.keys(naturalized)) {
-          const value = naturalized[key];
-
-          if (Array.isArray(value) && typeof value[0] === 'object') {
-            // Fix recursive values
-            const validValues = value.filter(Boolean);
-            validValues.forEach(child => addRetrieveBulkDataNaturalized(child, instance));
-            continue;
-          }
-
-          // The value.Value will be set with the bulkdata read value
-          // in which case it isn't necessary to re-read this.
-          if (value && value.BulkDataURI && !value.Value) {
-            // handle the scenarios where bulkDataURI is relative path
-            fixBulkDataURI(value, instance, dicomWebConfig);
-            // Provide a method to fetch bulkdata
-            value.retrieveBulkData = retrieveBulkData.bind(qidoDicomWebClient, value);
-          }
-        }
-        return naturalized;
-      };
-
-      /**
-       * naturalizes the dataset, and adds a retrieve bulkdata method
-       * to any values containing BulkDataURI.
-       * @param {*} instance
-       * @returns naturalized dataset, with retrieveBulkData methods
-       */
-      const addRetrieveBulkData = instance => {
-        const naturalized = naturalizeDataset(instance);
-
-        // if we know the server doesn't use bulkDataURI, then don't
-        if (!dicomWebConfig.bulkDataURI?.enabled) {
-          return naturalized;
-        }
-
-        return addRetrieveBulkDataNaturalized(naturalized);
-      };
-
       // Async load series, store as retrieved
-      function storeInstances(instances) {
+      async function storeInstances(instances) {
         const naturalizedInstances = instances.map(addRetrieveBulkData);
 
+        // Resolve the registered bulkdata tags (e.g. the Philips SUV Scale
+        // Factor) that the server delivered as bulkdata into plain numbers
+        // BEFORE INSTANCES_ADDED fires, so SUV scaling and every other
+        // subscriber read a fully-resolved value rather than an unresolved
+        // { BulkDataURI }. retrieveBulkData is bound to qidoDicomWebClient, so
+        // refresh its auth headers first (matching every other qido op here).
+        qidoDicomWebClient.headers = getAuthorizationHeader();
+        await utils.resolveBulkDataTags(naturalizedInstances);
+
         // Adding instanceMetadata to OHIF MetadataProvider
         naturalizedInstances.forEach(instance => {
           setNonEnumerableInstanceProperty(instance, 'wadoRoot', dicomWebConfig.wadoRoot);
@@ -678,20 +697,44 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) {
 
       DicomMetadataStore.addSeriesMetadata(seriesSummaryMetadata, madeInClient);
 
+      let completedSeriesCount = 0;
       const seriesDeliveredPromises = seriesPromises.map(promise => {
-        if (!returnPromises) {
-          promise?.start();
-        }
-        return promise.then(instances => {
-          storeInstances(instances);
-        });
+        let deliveredPromise;
+
+        return {
+          metadata: promise.metadata,
+          start: () => {
+            if (!deliveredPromise) {
+              deliveredPromise = promise.start().then(async instances => {
+                await storeInstances(instances);
+
+                completedSeriesCount++;
+                if (returnPromises && completedSeriesCount === seriesPromises.length) {
+                  setSuccessFlag();
+                }
+
+                return instances;
+              });
+            }
+
+            return deliveredPromise;
+          },
+        };
       });
 
       if (returnPromises) {
-        Promise.all(seriesDeliveredPromises).then(() => setSuccessFlag());
-        return seriesPromises;
+        if (!seriesDeliveredPromises.length) {
+          setSuccessFlag();
+        }
+
+        // The route starts only the series required by the hanging protocol,
+        // then starts the remainder in the background. Return wrappers whose
+        // start() resolves after async metadata post-processing has stored the
+        // instances and fired INSTANCES_ADDED; resolving the raw retrieval here
+        // races hanging-protocol application against display-set creation.
+        return seriesDeliveredPromises;
       } else {
-        await Promise.all(seriesDeliveredPromises);
+        await Promise.all(seriesDeliveredPromises.map(promise => promise.start()));
         setSuccessFlag();
       }
 
@@ -761,34 +804,4 @@ function createDicomWebApi(dicomWebConfig: DicomWebConfig, servicesManager) {
   return IWebApiDataSource.create(implementation);
 }
 
-/**
- * A bindable function that retrieves the bulk data against this as the
- * dicomweb client, and on the given value element.
- *
- * @param value - a bind value that stores the retrieve value to short circuit the
- *    next retrieve instance.
- * @param options - to allow specifying the content type.
- */
-function retrieveBulkData(value, options = {}) {
-  const { mediaType } = options;
-  const useOptions = {
-    // The bulkdata fetches work with either multipart or
-    // singlepart, so set multipart to false to let the server
-    // decide which type to respond with.
-    multipart: false,
-    BulkDataURI: value.BulkDataURI,
-    mediaTypes: mediaType ? [{ mediaType }, { mediaType: 'application/octet-stream' }] : undefined,
-    ...options,
-  };
-  return this.retrieveBulkData(useOptions).then(val => {
-    // There are DICOM PDF cases where the first ArrayBuffer in the array is
-    // the bulk data and DICOM video cases where the second ArrayBuffer is
-    // the bulk data. Here we play it safe and do a find.
-    const ret =
-      (val instanceof Array && val.find(arrayBuffer => arrayBuffer?.byteLength)) || undefined;
-    value.Value = ret;
-    return ret;
-  });
-}
-
 export { createDicomWebApi };

extensions/default/src/DicomWebDataSource/retrieveBulkData.test.ts

@@ -0,0 +1,35 @@
+import retrieveBulkData from './retrieveBulkData';
+
+describe('retrieveBulkData', () => {
+  it('returns and caches a single-part ArrayBuffer response', async () => {
+    const buffer = new Uint8Array([1]).buffer;
+    const client = {
+      retrieveBulkData: jest.fn().mockResolvedValue(buffer),
+    };
+    const value: { BulkDataURI: string; Value?: ArrayBuffer } = {
+      BulkDataURI: 'https://example.com/bulk/70531000',
+    };
+
+    await expect(retrieveBulkData.call(client, value)).resolves.toBe(buffer);
+    expect(value.Value).toBe(buffer);
+    expect(client.retrieveBulkData).toHaveBeenCalledWith(
+      expect.objectContaining({
+        multipart: false,
+        BulkDataURI: value.BulkDataURI,
+      })
+    );
+  });
+
+  it('still selects the non-empty buffer from a multipart response', async () => {
+    const buffer = new Uint8Array([2]).buffer;
+    const client = {
+      retrieveBulkData: jest.fn().mockResolvedValue([new ArrayBuffer(0), buffer]),
+    };
+    const value: { BulkDataURI: string; Value?: ArrayBuffer } = {
+      BulkDataURI: 'https://example.com/bulk/70531009',
+    };
+
+    await expect(retrieveBulkData.call(client, value)).resolves.toBe(buffer);
+    expect(value.Value).toBe(buffer);
+  });
+});

extensions/default/src/DicomWebDataSource/retrieveBulkData.ts

@@ -0,0 +1,31 @@
+/**
+ * A bindable function that retrieves bulkdata against `this` DICOMweb client
+ * and caches the resolved buffer on the given value element.
+ *
+ * @param value - A bound value that stores the retrieved buffer.
+ * @param options - Options such as the requested content type.
+ */
+export default function retrieveBulkData(value, options = {}) {
+  const { mediaType } = options;
+  const useOptions = {
+    // The bulkdata fetches work with either multipart or singlepart, so set
+    // multipart to false to let the server decide which type to respond with.
+    multipart: false,
+    BulkDataURI: value.BulkDataURI,
+    mediaTypes: mediaType ? [{ mediaType }, { mediaType: 'application/octet-stream' }] : undefined,
+    ...options,
+  };
+
+  return this.retrieveBulkData(useOptions).then(val => {
+    // Single-part clients return a bare ArrayBuffer. Multipart clients return
+    // an array; DICOM PDF/video payloads may occupy different positions, so
+    // select the first non-empty part in that response shape.
+    const ret = Array.isArray(val)
+      ? val.find(arrayBuffer => arrayBuffer?.byteLength)
+      : val?.byteLength
+        ? val
+        : undefined;
+    value.Value = ret;
+    return ret;
+  });
+}