From 92c00985204702ba5e39d6abf3af30216fa2e68f Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 22 Oct 2026 15:05:38 -0400 Subject: [PATCH 1/3] feat: bind additional image inputs to attached layers --- .../generated/job-history-detail.schema.json | 3 + backend-contract/generated/openapi.json | 3 + backend-contract/processing/wire.ts | 1 + src/processing/__tests__/job-adoption.spec.ts | 159 +++++++++++ src/processing/components/JobsModule.vue | 9 +- src/processing/components/TaskForm.vue | 16 ++ .../components/__tests__/JobsModule.spec.ts | 250 ++++++++++++++++++ .../components/widgets/FileWidget.vue | 12 +- .../widgets/__tests__/FileWidget.spec.ts | 35 +++ src/processing/composables/useInputStaging.ts | 12 + .../engine/__tests__/jobHistory.spec.ts | 47 ++++ .../engine/__tests__/mintInput.spec.ts | 12 +- src/processing/engine/jobDisplay.ts | 11 +- src/processing/engine/jobHistory.ts | 15 +- src/processing/engine/mintInput.ts | 94 +++++-- src/processing/engine/sourceRefs.ts | 8 +- src/processing/store.ts | 15 +- 17 files changed, 649 insertions(+), 53 deletions(-) create mode 100644 src/processing/__tests__/job-adoption.spec.ts diff --git a/backend-contract/generated/job-history-detail.schema.json b/backend-contract/generated/job-history-detail.schema.json index 44c80c1c1..f2af413ed 100644 --- a/backend-contract/generated/job-history-detail.schema.json +++ b/backend-contract/generated/job-history-detail.schema.json @@ -19,6 +19,9 @@ "type": "string" }, "additionalProperties": {} + }, + "referenceImageParameter": { + "type": "string" } }, "required": [ diff --git a/backend-contract/generated/openapi.json b/backend-contract/generated/openapi.json index 15816ab98..5865f7143 100644 --- a/backend-contract/generated/openapi.json +++ b/backend-contract/generated/openapi.json @@ -1577,6 +1577,9 @@ "type": "string" }, "additionalProperties": {} + }, + "referenceImageParameter": { + "type": "string" } }, "required": [ diff --git a/backend-contract/processing/wire.ts b/backend-contract/processing/wire.ts index df394d50b..09a1b309f 100644 --- a/backend-contract/processing/wire.ts +++ b/backend-contract/processing/wire.ts @@ -344,6 +344,7 @@ export const jobHistoryDetailSchema = z.object({ jobId: pathSegmentIdSchema, log: z.array(z.string()), parameters: z.record(z.string(), z.unknown()), + referenceImageParameter: z.string().optional(), }); export type JobHistoryDetail = z.infer; diff --git a/src/processing/__tests__/job-adoption.spec.ts b/src/processing/__tests__/job-adoption.spec.ts new file mode 100644 index 000000000..7ea1864c7 --- /dev/null +++ b/src/processing/__tests__/job-adoption.spec.ts @@ -0,0 +1,159 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; + +import { + appApplyDependencies, + autoLoadProcessingResults, +} from '@/src/processing/applyResults'; +import { useProcessingJobsStore } from '@/src/processing/store'; +import type { ProcessingResult } from '@/src/processing/types'; +import { + seatDataSource, + seatVolume, +} from '@/src/store/__tests__/datasetFixtures'; +import { seatImage } from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { serializeAnnotations } from './serializedScene'; + +const jobRef = { providerId: 'analysis', jobId: 'saved-job' }; +const baseUris = ['/ct/2.dcm', '/ct/1.dcm']; +const layerUris = ['/pet/1.dcm', '/pet/2.dcm']; + +const rulerFile = { + schemaVersion: 1, + space: 'LPS', + labels: { + rulers: { Tumor: { color: '#ff0000', strokeWidth: 3 } }, + rectangles: {}, + polygons: {}, + }, + tools: { + rulers: [ + { + name: 'Long axis', + labelName: 'Tumor', + firstPoint: [1, 1, 2], + secondPoint: [4, 1, 2], + frameOfReference: { + planeNormal: [0, 0, 1], + planeOrigin: [0, 0, 2], + }, + }, + ], + rectangles: [], + polygons: [], + }, +}; + +const result: ProcessingResult = { + intent: 'add-annotations', + id: 'rulers', + name: 'measurements.annotations.json', + url: '/measurements.annotations.json', + source: { ...jobRef, outputId: 'outputAnnotations' }, +}; + +const loadVolume = async (id: string, uris: string[]) => { + await seatImage(id, { dimensions: [8, 8, 8] }); + seatVolume(id); + seatDataSource(id, { + type: 'collection', + sources: uris.map((uri) => ({ type: 'uri', uri, name: uri })), + }); +}; + +beforeEach(() => { + setActivePinia(createPinia()); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('adopting jobs with base and layer inputs', () => { + it.each([ + { baseLoaded: true, baseSubmitted: true, expectedCount: 1 }, + { baseLoaded: false, baseSubmitted: true, expectedCount: 0 }, + { baseLoaded: true, baseSubmitted: false, expectedCount: 0 }, + ])( + 'restores only the named base with loaded=$baseLoaded, submitted=$baseSubmitted', + async ({ baseLoaded, baseSubmitted, expectedCount }) => { + if (baseLoaded) await loadVolume('base-ct', [...baseUris].reverse()); + await loadVolume('pet-layer', layerUris); + const detail = { + jobId: jobRef.jobId, + log: [], + parameters: { + absentLayer: null, + inputPETVolume: { type: 'image', uris: layerUris }, + inputTools: { type: 'annotations', uris: ['/tools.json'] }, + ...(baseSubmitted + ? { inputVolume: { type: 'image', uris: baseUris } } + : {}), + }, + referenceImageParameter: 'inputVolume', + }; + const history = { + jobs: [ + { + jobId: jobRef.jobId, + taskId: 'measure', + taskTitle: 'Measure segments', + createdBy: { id: 'reader', name: 'Reader' }, + createdAt: '2026-10-21T12:00:00Z', + state: 'success', + resultState: 'ready', + outputSummary: { recorded: 1, missing: 0 }, + }, + ], + nextCursor: null, + }; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL) => { + const path = new URL(String(input), window.location.origin).pathname; + let payload: unknown; + if (path.endsWith('/detail')) payload = detail; + else if (path.endsWith('/results')) + payload = { resultState: 'ready', intents: [result], missing: 0 }; + else if (path === '/analysis/jobs') payload = history; + else throw new Error(`Unexpected request: ${path}`); + return new Response(JSON.stringify(payload), { + headers: { 'Content-Type': 'application/json' }, + }); + }) + ); + const jobs = useProcessingJobsStore(); + jobs.registerProviderConfig({ + id: jobRef.providerId, + label: 'Analysis', + baseUrl: '/analysis', + jobsBaseUrl: '/analysis', + }); + + await jobs.adoptJobHistory(); + await jobs.loadJobResults(jobRef); + await jobs.applyJobResults(jobRef, (pending, submitted) => + autoLoadProcessingResults(pending, submitted, { + ...appApplyDependencies(), + fetchResult: async () => + new File([JSON.stringify(rulerFile)], result.name), + }) + ); + + const savedRulers = serializeAnnotations().tools?.rulers?.tools ?? []; + if (expectedCount) { + expect(savedRulers).toMatchObject([ + { + imageID: 'base-ct', + name: 'Long axis', + firstPoint: [1, 1, 2], + secondPoint: [4, 1, 2], + }, + ]); + expect(savedRulers).toHaveLength(1); + } else { + expect(savedRulers).toEqual([]); + } + } + ); +}); diff --git a/src/processing/components/JobsModule.vue b/src/processing/components/JobsModule.vue index 27698ad63..9843a798d 100644 --- a/src/processing/components/JobsModule.vue +++ b/src/processing/components/JobsModule.vue @@ -136,7 +136,10 @@ import { type TaskFormModel, type FormValidationIssue, } from '@/src/processing/engine/formModel'; -import { type SourceRefBindingState } from '@/src/processing/engine/mintInput'; +import { + mintInputValue, + type SourceRefBindingState, +} from '@/src/processing/engine/mintInput'; import { bindSourceRefs, type BoundSourceRefType, @@ -510,6 +513,7 @@ function jobDisplayContext(bindings: SourceRefBindings): JobDisplayContext { labelmapNames, types: bindings.types, imageName: activeImageName(), + imageNames: bindings.image.names ?? {}, annotationCount: finishedAnnotationCount.value, }; } @@ -583,6 +587,9 @@ watchDebounced( // Placing the first (or removing the last) tool flips the annotations // binding, so the form must revalidate. annotationCount: finishedAnnotationCount.value, + imageLayers: sourceRefContext().imageLayers?.map(({ dataSource }) => + mintInputValue(dataSource) + ), }; }, () => { diff --git a/src/processing/components/TaskForm.vue b/src/processing/components/TaskForm.vue index aea5c50cc..5888ab7fa 100644 --- a/src/processing/components/TaskForm.vue +++ b/src/processing/components/TaskForm.vue @@ -31,6 +31,7 @@ :binding="sourceRefStates?.[field.id]" :bound-name="sourceRefNames?.[field.id]" :bound-type="sourceRefTypes?.[field.id]" + :image-index="imageInputIndices[field.id]" :warning="sourceRefWarnings?.[field.id]" @update:model-value="(v: ProcessingValue) => update(field.id, v)" /> @@ -96,6 +97,7 @@ import { } from '@/src/processing/engine/formModel'; import type { SourceRefBindingState } from '@/src/processing/engine/mintInput'; import type { BoundSourceRefType } from '@/src/processing/engine/sourceRefs'; +import { TYPE_TAG_IMAGE } from '@/backend-contract'; import BooleanWidget from './widgets/BooleanWidget.vue'; import NumberWidget from './widgets/NumberWidget.vue'; @@ -166,6 +168,20 @@ const inputFields = computed(() => props.model.fields.filter((f) => f.kind === 'sourceRef') ); +const imageInputIndices = computed(() => + Object.fromEntries( + inputFields.value + .filter( + (field) => + props.sourceRefTypes?.[field.id] === TYPE_TAG_IMAGE || + (!props.sourceRefTypes?.[field.id] && + field.accepts.length === 1 && + field.accepts[0] === TYPE_TAG_IMAGE) + ) + .map((field, index) => [field.id, index]) + ) +); + function widgetFor(kind: FormField['kind']) { switch (kind) { case 'bool': diff --git a/src/processing/components/__tests__/JobsModule.spec.ts b/src/processing/components/__tests__/JobsModule.spec.ts index 2d4712237..def7fde5d 100644 --- a/src/processing/components/__tests__/JobsModule.spec.ts +++ b/src/processing/components/__tests__/JobsModule.spec.ts @@ -46,10 +46,13 @@ import TaskPicker from '@/src/processing/components/TaskPicker.vue'; import TaskForm from '@/src/processing/components/TaskForm.vue'; import { useProcessingJobsStore } from '@/src/processing/store'; import { useDatasetStore } from '@/src/store/datasets'; +import { useLayersStore } from '@/src/store/datasets-layers'; import { useRulerStore } from '@/src/store/tools/rulers'; import { useSegmentationStore } from '@/src/segmentation/store'; import { + activateAppPinia, seedVoxel, + seatImage, mintSegment, selectSegment, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; @@ -110,6 +113,11 @@ const mountJobsModule = (pinia: ReturnType) => 'v-select': true, TaskForm: false, FileWidget: false, + ReasonedAction: false, + 'v-btn': { + props: ['disabled'], + template: '', + }, 'v-expansion-panels': slotStub, 'v-expansion-panel': slotStub, 'v-expansion-panel-title': slotStub, @@ -530,6 +538,248 @@ describe('JobsModule segmentation staging', () => { return { provider: p, submitSpy }; }; + const orderedImageSpec = (requiredLayer = false): TaskSpecEnvelope => ({ + ...envelope('seg', 'Image report'), + parameters: [ + { + kind: 'sourceRef', + id: 'referenceScan', + accepts: ['image'], + required: true, + }, + { + kind: 'sourceRef', + id: 'auxiliaryScan', + accepts: ['image'], + required: requiredLayer, + }, + { kind: 'sourceRef', id: 'detailScan', accepts: ['image'] }, + ], + }); + + const seatVolume = async (id: string, name: string, slices = 2) => { + await seatImage(id, { name, dimensions: [2, 2, 2] }); + const uris = Array.from( + { length: slices }, + (_, index) => `girder://file/${id}-${index + 1}` + ); + useDatasetStore().addDataSources([ + { + dataID: id, + dataSource: { + type: 'collection', + sources: uris.map((uri, index) => ({ + type: 'uri' as const, + uri, + name: `${index + 1}.dcm`, + })), + }, + }, + ]); + return { type: 'image', uris }; + }; + + const mountImageReport = async (spec = orderedImageSpec()) => { + const p = stagingProvider(spec); + p.getJob = vi.fn(async (jobId: string) => ({ + jobId, + state: 'cancelled' as const, + resultState: 'unavailable' as const, + })); + registerFake(useProcessingJobsStore(), p); + const wrapper = mount(); + await flushPromises(); + return { p, wrapper, form: wrapper.findComponent(TaskForm) }; + }; + + const inputNames = ( + form: Awaited>['form'] + ) => form.findAll('.file-widget .value-text').map((value) => value.text()); + + it('binds each image parameter to one complete volume in base and layer order', async () => { + pinia = activateAppPinia(); + const referenceScan = await seatVolume('image-1', 'CT volume', 3); + useViewStore().setDataForAllViews('image-1'); + const auxiliaryScan = await seatVolume('pet-1', 'PET volume', 2); + const detailScan = await seatVolume('mr-1', 'MR volume', 4); + await seatVolume('unrelated', 'Unattached volume'); + const layers = useLayersStore(); + await layers.addLayer('image-1', 'pet-1'); + await layers.addLayer('image-1', 'mr-1'); + const { p, form } = await mountImageReport(); + + expect(inputNames(form)).toEqual(['CT volume', 'PET volume', 'MR volume']); + expect(form.get('button').attributes('disabled')).toBeUndefined(); + form.vm.$emit( + 'update:values', + Object.fromEntries(Object.entries(form.props('values')).reverse()) + ); + await flushPromises(); + await form.get('button').trigger('click'); + await flushPromises(); + expect(p.runTask).toHaveBeenCalledWith('seg', { + referenceScan, + auxiliaryScan, + detailScan, + }); + }); + + it('submits missing optional layers as null and excludes unrelated loaded volumes', async () => { + await seatVolume('unrelated-pet', 'Unattached PET volume'); + const { p, form } = await mountImageReport(); + + expect(inputNames(form)).toEqual([ + 'image.nrrd', + 'Not provided', + 'Not provided', + ]); + expect(form.get('button').attributes('disabled')).toBeUndefined(); + await form.get('button').trigger('click'); + await flushPromises(); + expect(p.runTask).toHaveBeenCalledWith('seg', { + referenceScan: { + type: 'image', + format: 'nrrd', + uris: ['girder://file/image-1'], + }, + auxiliaryScan: null, + detailScan: null, + }); + }); + + it('blocks submission when a required layer input is missing', async () => { + const { p, form } = await mountImageReport(orderedImageSpec(true)); + + expect(inputNames(form)[1]).not.toBe('Not provided'); + expect(form.get('button').attributes('disabled')).toBeDefined(); + await form.get('button').trigger('click'); + await flushPromises(); + expect(p.runTask).not.toHaveBeenCalled(); + }); + + it('shifts attached layers and clears stale values after removal', async () => { + await seatVolume('pet-1', 'PET volume'); + const remainingLayer = await seatVolume('mr-1', 'MR volume'); + const layers = useLayersStore(); + await layers.addLayer('image-1', 'pet-1'); + await layers.addLayer('image-1', 'mr-1'); + const { p, form } = await mountImageReport(); + expect(inputNames(form)).toEqual(['image.nrrd', 'PET volume', 'MR volume']); + + layers.deleteLayer('image-1', 'pet-1'); + await afterDebounce(); + expect(inputNames(form)).toEqual([ + 'image.nrrd', + 'MR volume', + 'Not provided', + ]); + await form.get('button').trigger('click'); + await flushPromises(); + expect(p.runTask).toHaveBeenLastCalledWith( + 'seg', + expect.objectContaining({ + auxiliaryScan: remainingLayer, + detailScan: null, + }) + ); + + layers.deleteLayer('image-1', 'mr-1'); + await afterDebounce(); + expect(inputNames(form)).toEqual([ + 'image.nrrd', + 'Not provided', + 'Not provided', + ]); + await form.get('button').trigger('click'); + await flushPromises(); + expect(p.runTask).toHaveBeenLastCalledWith( + 'seg', + expect.objectContaining({ auxiliaryScan: null, detailScan: null }) + ); + }); + + it('clears previous layer values when the active base volume changes', async () => { + await seatVolume('pet-1', 'PET volume'); + const referenceScan = await seatVolume('image-2', 'Second CT volume', 3); + await useLayersStore().addLayer('image-1', 'pet-1'); + const { p, form } = await mountImageReport(); + expect(inputNames(form)).toEqual([ + 'image.nrrd', + 'PET volume', + 'Not provided', + ]); + + useViewStore().setDataForAllViews('image-2'); + await afterDebounce(); + expect(inputNames(form)).toEqual([ + 'Second CT volume', + 'Not provided', + 'Not provided', + ]); + await form.get('button').trigger('click'); + await flushPromises(); + expect(p.runTask).toHaveBeenCalledWith('seg', { + referenceScan, + auxiliaryScan: null, + detailScan: null, + }); + }); + + it('stages segmentation and annotations against the base when image inputs also bind layers', async () => { + pinia = activateAppPinia(); + const referenceScan = await seatVolume('image-1', 'CT volume.nrrd', 3); + useViewStore().setDataForAllViews('image-1'); + const auxiliaryScan = await seatVolume('pet-1', 'PET volume'); + const detailScan = await seatVolume('mr-1', 'MR volume'); + const layers = useLayersStore(); + await layers.addLayer('image-1', 'pet-1'); + await layers.addLayer('image-1', 'mr-1'); + seedSegmentation('Tumor'); + finishRuler(); + const spec = orderedImageSpec(); + spec.parameters.splice(1, 0, { + kind: 'sourceRef', + id: 'regions', + accepts: ['labelmap'], + required: true, + }); + spec.parameters.splice(3, 0, { + kind: 'sourceRef', + id: 'measurements', + accepts: ['annotations'], + required: true, + }); + const { p, form } = await mountImageReport(spec); + + expect(form.get('button').attributes('disabled')).toBeUndefined(); + await form.get('button').trigger('click'); + await flushPromises(); + expect( + p.stageInput.mock.calls.map(([request]) => request.descriptor) + ).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'labelmap', + referenceImage: referenceScan, + }), + expect.objectContaining({ + type: 'annotations', + referenceImage: referenceScan, + }), + ]) + ); + expect(p.runTask).toHaveBeenCalledWith('seg', { + referenceScan, + auxiliaryScan, + detailScan, + regions: { type: 'labelmap', uris: ['girder://staged/Tumor.seg.nrrd'] }, + measurements: { + type: 'annotations', + uris: ['girder://staged/CT volume.annotations.json'], + }, + }); + }); + it('stages the image’s segmentation for a multiple param', async () => { seedSegmentation('Tumor'); diff --git a/src/processing/components/widgets/FileWidget.vue b/src/processing/components/widgets/FileWidget.vue index 81fec9af5..f8692f239 100644 --- a/src/processing/components/widgets/FileWidget.vue +++ b/src/processing/components/widgets/FileWidget.vue @@ -43,6 +43,7 @@ const props = defineProps<{ binding?: SourceRefBindingState; boundName?: string; boundType?: BoundSourceRefType; + imageIndex?: number; warning?: string; }>(); @@ -80,7 +81,15 @@ const kind = computed(() => { ? props.param.accepts[0] : undefined; const type = props.boundType ?? declared; - return (type ? KINDS[type] : undefined) ?? IMAGE_KIND; + const declaredKind = type ? KINDS[type] : undefined; + if (declaredKind) return declaredKind; + return props.imageIndex && props.imageIndex > 0 + ? { + ...IMAGE_KIND, + caption: `Image layer ${props.imageIndex}`, + noun: 'layer' as const, + } + : IMAGE_KIND; }); const OPTIONAL_UNBOUND_STATES = new Set([ @@ -88,6 +97,7 @@ const OPTIONAL_UNBOUND_STATES = new Set([ 'no-segmentation', 'no-annotations', 'no-reference-input', + 'no-layer', ]); // Missing optional inputs are an intentional task state, not an instruction diff --git a/src/processing/components/widgets/__tests__/FileWidget.spec.ts b/src/processing/components/widgets/__tests__/FileWidget.spec.ts index 949c2ef2e..aaddba9da 100644 --- a/src/processing/components/widgets/__tests__/FileWidget.spec.ts +++ b/src/processing/components/widgets/__tests__/FileWidget.spec.ts @@ -135,3 +135,38 @@ describe('FileWidget segmentation inputs', () => { } ); }); + +describe('FileWidget ordered image inputs', () => { + it('labels a layer slot and shows its bound volume name', () => { + const wrapper = mountLabelmap(labelmapParam({ accepts: ['image'] }), { + binding: 'bound', + imageIndex: 2, + boundName: 'Perfusion volume', + }); + + expect(wrapper.get('.key-text').text()).toBe('Image layer 2'); + expect(wrapper.get('.input-value').text()).toBe('Perfusion volume'); + }); + + it.each([false, true])( + 'renders an absent layer with required=%s', + (required) => { + const wrapper = mountLabelmap( + labelmapParam({ accepts: ['image'], required }), + { + binding: 'no-layer', + imageIndex: 1, + } + ); + + expect(wrapper.get('.input-value').text()).toBe( + required + ? 'Attach another image layer to the active dataset first.' + : 'Not provided' + ); + expect(wrapper.get('.input-value').classes().includes('text-error')).toBe( + required + ); + } + ); +}); diff --git a/src/processing/composables/useInputStaging.ts b/src/processing/composables/useInputStaging.ts index d8614f08f..e9b8b6bcf 100644 --- a/src/processing/composables/useInputStaging.ts +++ b/src/processing/composables/useInputStaging.ts @@ -10,6 +10,7 @@ import { planSegmentationInput } from '@/src/processing/segmentationInput'; import { useCurrentImage } from '@/src/composables/useCurrentImage'; import { useImageCacheStore } from '@/src/store/image-cache'; import { useDatasetStore } from '@/src/store/datasets'; +import { useLayersStore } from '@/src/store/datasets-layers'; import { useSegmentationStore } from '@/src/segmentation/store'; import { segmentationHasContent } from '@/src/segmentation/model'; import { useSegmentStore } from '@/src/segmentation/segments'; @@ -65,6 +66,7 @@ export function useInputStaging(write = writeSegmentation) { const { currentImageID } = useCurrentImage('global'); const imageCache = useImageCacheStore(); const datasetStore = useDatasetStore(); + const layersStore = useLayersStore(); const segmentationStore = useSegmentationStore(); const activeDataSource = () => @@ -138,9 +140,19 @@ export function useInputStaging(write = writeSegmentation) { // segments all leave such a record behind. return { activeDataSource: activeDataSource(), + activeImageName: activeImageName(), segmentationId: record && segmentationHasContent(record) ? record.id : undefined, hasFinishedAnnotations: finishedAnnotationCount.value > 0, + imageLayers: layersStore + .getLayers(currentImageId) + .map(({ selection }) => ({ + dataSource: datasetStore.getDataSource(selection), + name: + imageCache.getImageMetadata(selection)?.name ?? + getDataSourceName(datasetStore.getDataSource(selection)) ?? + undefined, + })), }; }; diff --git a/src/processing/engine/__tests__/jobHistory.spec.ts b/src/processing/engine/__tests__/jobHistory.spec.ts index 8674ba801..cf8e2d062 100644 --- a/src/processing/engine/__tests__/jobHistory.spec.ts +++ b/src/processing/engine/__tests__/jobHistory.spec.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import { filterJobHistory, selectJobHistoryRows, + selectParentImageUris, type JobHistoryFilters, } from '@/src/processing/engine/jobHistory'; import type { JobHistorySummary } from '@/backend-contract'; @@ -31,6 +32,52 @@ const filter = ( overrides: Partial ) => filterJobHistory(jobs, overrides); +describe('job history reference image', () => { + const parameters = { + absentLayer: null, + inputPETVolume: { type: 'image', uris: ['/pet/1.dcm', '/pet/2.dcm'] }, + inputTools: { type: 'annotations', uris: ['/tools.json'] }, + inputVolume: { type: 'image', uris: ['/ct/2.dcm', '/ct/1.dcm'] }, + }; + + it('selects the recorded reference with all DICOM slices regardless of key order', () => { + expect(selectParentImageUris(parameters, 'inputVolume')).toEqual([ + '/ct/2.dcm', + '/ct/1.dcm', + ]); + }); + + it.each(['missing', 'absentLayer', 'inputTools'])( + 'does not fall back to another image when reference %s is unavailable', + (reference) => { + expect(selectParentImageUris(parameters, reference)).toBeUndefined(); + } + ); + + it('does not guess a reference for old jobs with multiple images', () => { + expect(selectParentImageUris(parameters)).toBeUndefined(); + }); + + it('recovers an old job with exactly one image input', () => { + expect( + selectParentImageUris({ + ...parameters, + inputPETVolume: null, + }) + ).toEqual(['/ct/2.dcm', '/ct/1.dcm']); + }); + + it('does not select a staged resource when no image input was supplied', () => { + expect( + selectParentImageUris({ + inputVolume: null, + inputLabelmap: { type: 'labelmap', uris: ['/mask.seg.nrrd'] }, + inputTools: { type: 'annotations', uris: ['/tools.json'] }, + }) + ).toBeUndefined(); + }); +}); + describe('job history filters', () => { const jobs = [ summary(), diff --git a/src/processing/engine/__tests__/mintInput.spec.ts b/src/processing/engine/__tests__/mintInput.spec.ts index bbbd6204e..425c61bea 100644 --- a/src/processing/engine/__tests__/mintInput.spec.ts +++ b/src/processing/engine/__tests__/mintInput.spec.ts @@ -202,7 +202,7 @@ describe('image binding auto-binds the active dataset', () => { expect(bindings.issues).toHaveLength(1); }); - it('fails closed (ambiguous) when more than one image param is present', () => { + it('binds the base and reports a missing required layer independently', () => { const model: TaskFormModel = { id: 'task', title: 'Task', @@ -216,10 +216,14 @@ describe('image binding auto-binds the active dataset', () => { model, context(remoteFile('/api/x/scan.nrrd', 'scan.nrrd')) ); - expect(bindings.states.ct).toBe('ambiguous'); - expect(bindings.states.pet).toBe('ambiguous'); - expect(bindings.image.values).toEqual({ ct: null, pet: null }); + expect(bindings.states.ct).toBe('bound'); + expect(bindings.states.pet).toBe('no-layer'); + expect(bindings.image.values).toEqual({ + ct: { type: 'image', format: 'nrrd', uris: ['/api/x/scan.nrrd'] }, + pet: null, + }); expect(bindings.issues).toHaveLength(1); + expect(bindings.issues[0].parameter).toBe('pet'); }); it('is a no-op when the task has no image input', () => { diff --git a/src/processing/engine/jobDisplay.ts b/src/processing/engine/jobDisplay.ts index 909978e6d..db47b97ba 100644 --- a/src/processing/engine/jobDisplay.ts +++ b/src/processing/engine/jobDisplay.ts @@ -19,6 +19,7 @@ export type JobDisplayContext = { labelmapNames: Record; types: Record; imageName: string | undefined; + imageNames: Record; annotationCount: number; }; @@ -36,7 +37,7 @@ const boundSourceRefName = ( const type = ctx.types[parameterId]; if (type === TYPE_TAG_LABELMAP) return ctx.labelmapNames[parameterId]; if (type === TYPE_TAG_ANNOTATIONS) return boundAnnotationsName(ctx); - return ctx.imageName; + return ctx.imageNames[parameterId]; }; export const buildSourceRefNames = ( @@ -58,13 +59,7 @@ export const formatProcessingValue = ( value: ProcessingValue ): string => { if (field.kind === 'sourceRef') { - // A bound labelmap param always names its segmentation, so the fallback is - // the optional param that bound nothing. - const fallback = - ctx.types[field.id] === TYPE_TAG_LABELMAP - ? 'not provided' - : 'active dataset'; - return boundSourceRefName(ctx, field.id) ?? fallback; + return boundSourceRefName(ctx, field.id) ?? 'not provided'; } if (field.kind === 'bounds') { return Array.isArray(value) && value.length > 0 diff --git a/src/processing/engine/jobHistory.ts b/src/processing/engine/jobHistory.ts index 994bd865a..a8c47b590 100644 --- a/src/processing/engine/jobHistory.ts +++ b/src/processing/engine/jobHistory.ts @@ -27,13 +27,18 @@ function isImageInputValue(v: unknown): v is { type: string; uris: string[] } { return parsed.success && !STAGED_INPUT_TYPES.has(parsed.data.type); } -// Anything but exactly one image input is ambiguous; never guess a parent to -// attach results to. +// A saved parameter name identifies the reference independently of JSON key +// order. Older jobs without it are unambiguous only with one image input. export const selectParentImageUris = ( - parameters: JobHistoryDetail['parameters'] + parameters: JobHistoryDetail['parameters'], + referenceImageParameter?: string ): string[] | undefined => { - const imageInputs = Object.values(parameters ?? {}).filter(isImageInputValue); - return imageInputs.length === 1 ? imageInputs[0].uris : undefined; + if (referenceImageParameter !== undefined) { + const value = parameters?.[referenceImageParameter]; + return isImageInputValue(value) ? value.uris : undefined; + } + const images = Object.values(parameters ?? {}).filter(isImageInputValue); + return images.length === 1 ? images[0].uris : undefined; }; // Rows key on (providerId, jobId) because two providers may share a raw jobId. diff --git a/src/processing/engine/mintInput.ts b/src/processing/engine/mintInput.ts index 98f18a519..858b76f1f 100644 --- a/src/processing/engine/mintInput.ts +++ b/src/processing/engine/mintInput.ts @@ -62,11 +62,12 @@ export type SourceRefBindingState = | 'no-segmentation' | 'no-annotations' | 'no-reference-input' + | 'no-layer' | 'ambiguous'; // What a binder calls the thing it could not bind, in the user-facing // sentences below. -export type SourceRefNoun = 'image' | 'segmentation' | 'annotation'; +export type SourceRefNoun = 'image' | 'segmentation' | 'annotation' | 'layer'; export type SourceRefField = Extract; @@ -89,7 +90,11 @@ export const bindingStateMessage = ( noun: SourceRefNoun ): string | undefined => { if (state === 'no-provenance') - return 'The active volume was not loaded from the server, so it cannot be used as an input.'; + return noun === 'layer' + ? 'The image layer was not loaded from the server, so it cannot be used as an input.' + : 'The active volume was not loaded from the server, so it cannot be used as an input.'; + if (state === 'no-layer') + return 'Attach another image layer to the active dataset first.'; if (state === 'no-segmentation') return 'Paint a segment on the current image first.'; if (state === 'no-annotations') @@ -146,6 +151,7 @@ export const unboundBinding = ( export type ImageBindingResult = { values: Record; states: Record; + names?: Record; // Caller suppresses generic duplicates for these param ids. issues: FormValidationIssue[]; }; @@ -156,26 +162,23 @@ const EMPTY_BINDING: ImageBindingResult = { issues: [], }; -const bindImageFields = ( - fields: SourceRefField[], - activeDataSource: DataSource | undefined, - value: InputValue | null -): ImageBindingResult => { - if (fields.length === 0) return EMPTY_BINDING; - - // Explicit null, not `{}`: a stale merge could submit the previous image. - const nullValues = () => - Object.fromEntries(fields.map((f) => [f.id, null as ProcessingValue])); - - if (fields.length > 1) { - return { values: nullValues(), ...ambiguousBinding(fields, 'image') }; - } - - const [field] = fields; +export type ImageLayerInput = { + dataSource: DataSource | undefined; + name: string | undefined; +}; - if (!activeDataSource) { +const bindImageField = ( + field: SourceRefField, + source: ImageLayerInput | undefined, + value: InputValue | null, + isLayer: boolean +): ImageBindingResult => { + const values = { [field.id]: null }; + if (!source) { + if (isLayer) + return { values, ...unboundBinding(field, 'no-layer', 'layer') }; return { - values: nullValues(), + values, states: { [field.id]: 'unbound' }, issues: field.required ? [{ parameter: field.id, message: `${fieldLabel(field)} is required` }] @@ -187,8 +190,13 @@ const bindImageFields = ( // used as input. if (!value) { return { - values: nullValues(), - ...unboundBinding(field, 'no-provenance', 'image', true), + values, + ...unboundBinding( + field, + 'no-provenance', + isLayer ? 'layer' : 'image', + true + ), }; } @@ -202,6 +210,42 @@ const bindImageFields = ( export const bindMintedImageInputs = ( model: TaskFormModel, activeDataSource: DataSource | undefined, - value: InputValue | null -): ImageBindingResult => - bindImageFields(imageInputFields(model), activeDataSource, value); + activeValue: InputValue | null, + sources: { activeName?: string; layers?: readonly ImageLayerInput[] } = {} +): ImageBindingResult => { + const fields = imageInputFields(model); + if (fields.length === 0) return EMPTY_BINDING; + const images = [ + activeDataSource + ? { dataSource: activeDataSource, name: sources.activeName } + : undefined, + ...(sources.layers ?? []), + ]; + // A collection is one volume; its complete URI set stays within one field. + const bindings = fields.map((field, index) => + bindImageField( + field, + images[index], + index === 0 ? activeValue : mintInputValue(images[index]?.dataSource), + index > 0 + ) + ); + return { + values: Object.assign({}, ...bindings.map((binding) => binding.values)), + states: Object.assign({}, ...bindings.map((binding) => binding.states)), + names: Object.fromEntries( + fields.flatMap((field, index) => { + const image = images[index]; + if (!image) return []; + return [ + [ + field.id, + image.name ?? + (index === 0 ? 'Active dataset' : `Image layer ${index}`), + ], + ]; + }) + ), + issues: bindings.flatMap((binding) => binding.issues), + }; +}; diff --git a/src/processing/engine/sourceRefs.ts b/src/processing/engine/sourceRefs.ts index 15b96f44a..8d4deffb6 100644 --- a/src/processing/engine/sourceRefs.ts +++ b/src/processing/engine/sourceRefs.ts @@ -11,6 +11,7 @@ import { type ImageBindingResult, type SourceRefBindingState, type SourceRefField, + type ImageLayerInput, } from '@/src/processing/engine/mintInput'; import { bindLabelmapInputs, @@ -37,10 +38,12 @@ export type SourceRefBindings = { export type SourceRefBindingContext = { activeDataSource: DataSource | undefined; + activeImageName?: string; // The current image's segmentation, when it holds a mask. segmentationId: string | undefined; // Whether the active image carries at least one finished annotation tool. hasFinishedAnnotations: boolean; + imageLayers?: readonly ImageLayerInput[]; }; const BOUND_TYPES = new Set([ @@ -76,7 +79,7 @@ export const bindSourceRefs = ( const fields = model.fields.filter( (field): field is SourceRefField => field.kind === 'sourceRef' ); - // All supported source references share the current image provenance. + // Segmentations and annotations share the current image's provenance. const imageValue = fields.some((field) => acceptedTypes(field).length > 0) ? mintInputValue(context.activeDataSource, TYPE_TAG_IMAGE) : null; @@ -116,7 +119,8 @@ export const bindSourceRefs = ( const image = bindMintedImageInputs( modelForType(model, types, TYPE_TAG_IMAGE), context.activeDataSource, - imageValue + imageValue, + { activeName: context.activeImageName, layers: context.imageLayers } ); // The segmentation's reference image is the active image. const labelmap = bindLabelmapInputs( diff --git a/src/processing/store.ts b/src/processing/store.ts index 0c6dacae6..0204c9731 100644 --- a/src/processing/store.ts +++ b/src/processing/store.ts @@ -475,13 +475,14 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { provider: ProcessingProvider, jobId: string ): Promise { - let detail: JobHistoryDetail | undefined; - try { - detail = await provider.getJobHistoryDetail(jobId); - } catch { - return undefined; // best-effort: the open-as-dataset fallback still works - } - const uris = selectParentImageUris(detail?.parameters); + // History is best-effort; results can still open as standalone datasets. + const detail = await provider + .getJobHistoryDetail(jobId) + .catch(() => undefined); + const uris = selectParentImageUris( + detail?.parameters ?? {}, + detail?.referenceImageParameter + ); return uris === undefined ? undefined : datasetIdForUris(uris); } From d0b6a21b36c6396cea36dc804200c66139fce905 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 22 Oct 2026 15:42:07 -0400 Subject: [PATCH 2/3] fix: preserve results when job history is unavailable --- src/processing/__tests__/store.spec.ts | 31 ++++++++++++++++++++++++++ src/processing/store.ts | 27 +++++++++++----------- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/processing/__tests__/store.spec.ts b/src/processing/__tests__/store.spec.ts index fd0e020d0..9028f55b8 100644 --- a/src/processing/__tests__/store.spec.ts +++ b/src/processing/__tests__/store.spec.ts @@ -1514,6 +1514,37 @@ describe('Providers store — re-discovered job history: slim observability adop expect(store.jobResultsApplied.size).toBe(0); }); + it.each([ + [ + 'throws synchronously', + () => { + throw new Error('History unavailable'); + }, + ], + ['rejects', () => Promise.reject(new Error('History unavailable'))], + ] as const)( + 'loads adopted results when job detail %s', + async (_, readDetail) => { + const provider = makeProvider({ + listJobHistory: vi.fn().mockResolvedValue({ + jobs: [handle({ state: 'success' })], + nextCursor: null, + }), + getJobHistoryDetail: vi.fn(readDetail), + getResults: vi.fn().mockResolvedValue(resultsBundle(sampleResults)), + }); + const store = arrange(provider); + + await store.adoptJobHistory(); + await store.loadJobResults(ref('jr')); + + expect(store.jobResults.get(keyFor('jr'))).toEqual(sampleResults); + expect( + store.submittedContexts.get(keyFor('jr'))?.activeDatasetId + ).toBeUndefined(); + } + ); + it('a job that settles between listing and the first poll completes once', async () => { const provider = makeProvider({ listJobHistory: vi.fn().mockResolvedValue({ diff --git a/src/processing/store.ts b/src/processing/store.ts index 0204c9731..eaaa0b807 100644 --- a/src/processing/store.ts +++ b/src/processing/store.ts @@ -35,9 +35,6 @@ export const MAX_POLL_RETRIES = 4; export const MAX_POLL_BACKOFF_MS = 30000; export const MAX_JOB_HISTORY_PAGES = 1000; -const completionReady = (status: ProcessingJobStatus): boolean => - isTerminalJobState(status.state); - type PollErrorKind = | 'transient' | 'permanent' @@ -476,14 +473,16 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { jobId: string ): Promise { // History is best-effort; results can still open as standalone datasets. - const detail = await provider - .getJobHistoryDetail(jobId) - .catch(() => undefined); - const uris = selectParentImageUris( - detail?.parameters ?? {}, - detail?.referenceImageParameter - ); - return uris === undefined ? undefined : datasetIdForUris(uris); + try { + const detail = await provider.getJobHistoryDetail(jobId); + const uris = selectParentImageUris( + detail?.parameters ?? {}, + detail?.referenceImageParameter + ); + return uris === undefined ? undefined : datasetIdForUris(uris); + } catch { + return undefined; + } } // Rebuild an adopted job's missing parent id so a labelmap result attaches @@ -625,7 +624,7 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { if (!isCurrent(key, gen)) return; pollRetries.delete(key); recordJob(provider.config.id, status); - if (completionReady(status)) { + if (isTerminalJobState(status.state)) { stopPolling(key); await fireCompletion(provider, status); return; @@ -660,7 +659,7 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { ? { ...jobRef.status, jobId } : { jobId, state: 'pending', resultState: 'waiting' }; recordJob(providerId, initialStatus); - if (completionReady(initialStatus)) { + if (isTerminalJobState(initialStatus.state)) { await fireCompletion(provider, initialStatus); return jobId; } @@ -902,7 +901,7 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { } if (!isCurrent(key, gen)) return; recordJob(providerId, status); - if (completionReady(status)) { + if (isTerminalJobState(status.state)) { await fireCompletion(provider, status); return; } From cc4a129b55cba0baf81aabe2530243e3e0d1b2ac Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 22 Oct 2026 16:32:37 -0400 Subject: [PATCH 3/3] fix: show declared processing input roles --- .../components/__tests__/JobsModule.spec.ts | 27 +++++++++++++++++++ .../components/widgets/FileWidget.vue | 25 ++++++++++++++--- .../widgets/__tests__/FileWidget.spec.ts | 17 +++++++----- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/processing/components/__tests__/JobsModule.spec.ts b/src/processing/components/__tests__/JobsModule.spec.ts index def7fde5d..8cd8a96ca 100644 --- a/src/processing/components/__tests__/JobsModule.spec.ts +++ b/src/processing/components/__tests__/JobsModule.spec.ts @@ -596,6 +596,33 @@ describe('JobsModule segmentation staging', () => { form: Awaited>['form'] ) => form.findAll('.file-widget .value-text').map((value) => value.text()); + it('shows declared image roles beside their bound datasets before submission', async () => { + await seatVolume('follow-up', 'Follow-up volume'); + await useLayersStore().addLayer('image-1', 'follow-up'); + const spec = orderedImageSpec(); + spec.parameters = spec.parameters.map((parameter, index) => ({ + ...(parameter as object), + ...(index < 2 + ? { title: index === 0 ? 'Fixed image' : 'Moving image' } + : {}), + })); + const { p, form } = await mountImageReport(spec); + const inputs = form.findAll('.file-widget'); + + expect(inputNames(form)).toEqual([ + 'image.nrrd', + 'Follow-up volume', + 'Not provided', + ]); + expect(inputs[0].text()).toContain('Fixed image'); + expect(inputs[1].text()).toContain('Moving image'); + expect(inputs[2].text()).toContain('detailScan'); + expect(inputs[0].text()).toContain('Active dataset'); + expect(inputs[1].text()).toContain('Image layer 1'); + expect(inputs[1].text()).toContain('(optional)'); + expect(p.runTask).not.toHaveBeenCalled(); + }); + it('binds each image parameter to one complete volume in base and layer order', async () => { pinia = activateAppPinia(); const referenceScan = await seatVolume('image-1', 'CT volume', 3); diff --git a/src/processing/components/widgets/FileWidget.vue b/src/processing/components/widgets/FileWidget.vue index f8692f239..de3a42557 100644 --- a/src/processing/components/widgets/FileWidget.vue +++ b/src/processing/components/widgets/FileWidget.vue @@ -2,8 +2,15 @@
{{ kind.icon }} - - {{ kind.caption }}{{ param.required ? '' : ' (optional)' }} + + + {{ fieldLabel(param) }}{{ param.required ? '' : ' (optional)' }} + + {{ kind.caption }}