From e7da5d6bbda813d24ec811fc4c6e0e660e4ee9e7 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 27 Jul 2026 20:23:28 -0400 Subject: [PATCH 1/4] feat(processing): define the vector annotation contract Add a versioned world-LPS interchange format for rulers, rectangles, and polygons. Validate geometry and label references with strict wire fixtures. Extend staged inputs to a discriminated union of stageable types, then regenerate the artifacts. --- backend-contract/README.md | 17 +- .../annotations-bad-schema-version.json | 16 + .../negative/annotations-bad-space.json | 16 + .../negative/annotations-dangling-label.json | 24 + .../negative/annotations-session-field.json | 18 + .../annotations-two-point-polygon.json | 18 + .../negative/annotations-zero-normal.json | 16 + .../negative/stage-input-unknown-type.json | 10 + .../fixtures/wire/annotations-file.json | 72 +++ .../wire/stage-input.annotations.json | 12 + .../generated/annotations-file.schema.json | 422 ++++++++++++++ backend-contract/generated/openapi.json | 526 +++++++++++++++++- .../stage-input-descriptor.schema.json | 101 +++- .../processing/__tests__/annotations.spec.ts | 442 +++++++++++++++ .../processing/__tests__/wire.spec.ts | 41 ++ backend-contract/processing/annotations.ts | 242 ++++++++ backend-contract/processing/index.ts | 1 + backend-contract/processing/openapi.ts | 14 +- backend-contract/processing/schema-json.ts | 74 ++- backend-contract/processing/task-spec.ts | 10 +- backend-contract/processing/wire.ts | 60 +- 21 files changed, 2060 insertions(+), 92 deletions(-) create mode 100644 backend-contract/fixtures/negative/annotations-bad-schema-version.json create mode 100644 backend-contract/fixtures/negative/annotations-bad-space.json create mode 100644 backend-contract/fixtures/negative/annotations-dangling-label.json create mode 100644 backend-contract/fixtures/negative/annotations-session-field.json create mode 100644 backend-contract/fixtures/negative/annotations-two-point-polygon.json create mode 100644 backend-contract/fixtures/negative/annotations-zero-normal.json create mode 100644 backend-contract/fixtures/negative/stage-input-unknown-type.json create mode 100644 backend-contract/fixtures/wire/annotations-file.json create mode 100644 backend-contract/fixtures/wire/stage-input.annotations.json create mode 100644 backend-contract/generated/annotations-file.schema.json create mode 100644 backend-contract/processing/__tests__/annotations.spec.ts create mode 100644 backend-contract/processing/annotations.ts diff --git a/backend-contract/README.md b/backend-contract/README.md index bd38ff58e..20e03d85c 100644 --- a/backend-contract/README.md +++ b/backend-contract/README.md @@ -22,8 +22,11 @@ backend-contract/ index.ts top-level barrel; re-exports ./processing processing/ task-spec.ts VolView's zod task-spec schema - wire.ts neutral wire shapes: input value, job status, job history, - result-intent vocabulary + wire.ts neutral wire shapes: input value, staged-input + descriptor, job status, job history, result-intent + vocabulary + annotations.ts the annotations interchange FILE format (rulers, + rectangles, polygons) in world LPS mm openapi.ts the REST surface as an OpenAPI 3.1 document (wire schemas injected from the zod codegen) schema-json.ts zod -> JSON Schema codegen @@ -52,8 +55,12 @@ Task specs require two validation passes. First validate the generated `task-spec.schema.json`, then enforce the cross-field rules implemented by `validateTaskSpecSemantics` (or an equivalent implementation in the backend's language). Standard JSON Schema cannot compare sibling values such as -`default <= max`. Backend conformance tests must also assert that every payload -under `fixtures/negative/` is rejected by the combined validation path. +`default <= max`. The annotations interchange file needs the same two passes: +validate `annotations-file.schema.json`, then enforce +`validateAnnotationsFileSemantics` — every nonempty `labelName` must be declared +in its own tool-kind label namespace, which JSON Schema cannot express either. +Backend conformance tests must also assert that every payload under +`fixtures/negative/` is rejected by the combined validation path. ## The neutral REST surface (OpenAPI) @@ -69,7 +76,7 @@ enums. | `listJobHistory` | `GET /jobs` | → paged `JobHistorySummary[]` (optional) | | `getJobHistoryDetail` | `GET /jobs/{jobId}/detail` | → logs + submitted parameters on demand | | `deleteJob` | `DELETE /jobs/{jobId}` | → cascading deletion: execution record, results, staged inputs (terminal jobs only; 409 otherwise) | -| `stageInput` | `POST /stage` | parent-bound labelmap multipart → `StageResponse` (optional) | +| `stageInput` | `POST /stage` | parent-bound labelmap or annotations multipart → `StageResponse` (optional) | | `getJob` | `GET /jobs/{jobId}` | → `NeutralJobStatus` | | `getJobResults` | `GET /jobs/{jobId}/results` | → result intents, or explicit error | | `cancelJob` | `POST /jobs/{jobId}/cancel` | → `NeutralJobStatus` | diff --git a/backend-contract/fixtures/negative/annotations-bad-schema-version.json b/backend-contract/fixtures/negative/annotations-bad-schema-version.json new file mode 100644 index 000000000..0f437b840 --- /dev/null +++ b/backend-contract/fixtures/negative/annotations-bad-schema-version.json @@ -0,0 +1,16 @@ +{ + "schemaVersion": 2, + "space": "LPS", + "tools": { + "rulers": [ + { + "firstPoint": [0, 0, 0], + "secondPoint": [1, 1, 0], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, 0] + } + } + ] + } +} diff --git a/backend-contract/fixtures/negative/annotations-bad-space.json b/backend-contract/fixtures/negative/annotations-bad-space.json new file mode 100644 index 000000000..55abdedd2 --- /dev/null +++ b/backend-contract/fixtures/negative/annotations-bad-space.json @@ -0,0 +1,16 @@ +{ + "schemaVersion": 1, + "space": "RAS", + "tools": { + "rulers": [ + { + "firstPoint": [0, 0, 0], + "secondPoint": [1, 1, 0], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, 0] + } + } + ] + } +} diff --git a/backend-contract/fixtures/negative/annotations-dangling-label.json b/backend-contract/fixtures/negative/annotations-dangling-label.json new file mode 100644 index 000000000..6eee73a01 --- /dev/null +++ b/backend-contract/fixtures/negative/annotations-dangling-label.json @@ -0,0 +1,24 @@ +{ + "schemaVersion": 1, + "space": "LPS", + "labels": { + "rectangles": { + "lesion": { + "color": "#00ff00" + } + } + }, + "tools": { + "rulers": [ + { + "firstPoint": [0, 0, 0], + "secondPoint": [1, 1, 0], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, 0] + }, + "labelName": "lesion" + } + ] + } +} diff --git a/backend-contract/fixtures/negative/annotations-session-field.json b/backend-contract/fixtures/negative/annotations-session-field.json new file mode 100644 index 000000000..a0fa6e0a7 --- /dev/null +++ b/backend-contract/fixtures/negative/annotations-session-field.json @@ -0,0 +1,18 @@ +{ + "schemaVersion": 1, + "space": "LPS", + "tools": { + "rulers": [ + { + "firstPoint": [0, 0, 0], + "secondPoint": [1, 1, 0], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, 0] + }, + "imageID": "session-only-image-handle", + "color": "#ff0000" + } + ] + } +} diff --git a/backend-contract/fixtures/negative/annotations-two-point-polygon.json b/backend-contract/fixtures/negative/annotations-two-point-polygon.json new file mode 100644 index 000000000..1a4e97e4a --- /dev/null +++ b/backend-contract/fixtures/negative/annotations-two-point-polygon.json @@ -0,0 +1,18 @@ +{ + "schemaVersion": 1, + "space": "LPS", + "tools": { + "polygons": [ + { + "points": [ + [0, 0, 0], + [10, 0, 0] + ], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, 0] + } + } + ] + } +} diff --git a/backend-contract/fixtures/negative/annotations-zero-normal.json b/backend-contract/fixtures/negative/annotations-zero-normal.json new file mode 100644 index 000000000..77c061518 --- /dev/null +++ b/backend-contract/fixtures/negative/annotations-zero-normal.json @@ -0,0 +1,16 @@ +{ + "schemaVersion": 1, + "space": "LPS", + "tools": { + "rulers": [ + { + "firstPoint": [0, 0, 0], + "secondPoint": [1, 1, 0], + "frameOfReference": { + "planeNormal": [0, 0, 0], + "planeOrigin": [0, 0, 0] + } + } + ] + } +} diff --git a/backend-contract/fixtures/negative/stage-input-unknown-type.json b/backend-contract/fixtures/negative/stage-input-unknown-type.json new file mode 100644 index 000000000..a79ce252c --- /dev/null +++ b/backend-contract/fixtures/negative/stage-input-unknown-type.json @@ -0,0 +1,10 @@ +{ + "type": "pointset", + "name": "landmarks.json", + "referenceImage": { + "type": "image", + "uris": [ + "/api/v1/file/aaaaaaaaaaaaaaaaaaaaaaaa/proxiable/001.dcm" + ] + } +} diff --git a/backend-contract/fixtures/wire/annotations-file.json b/backend-contract/fixtures/wire/annotations-file.json new file mode 100644 index 000000000..7547ba974 --- /dev/null +++ b/backend-contract/fixtures/wire/annotations-file.json @@ -0,0 +1,72 @@ +{ + "schemaVersion": 1, + "space": "LPS", + "labels": { + "rulers": { + "lesion": { + "color": "#ff0000", + "strokeWidth": 2 + } + }, + "rectangles": { + "lesion": { + "color": "#00ff00", + "strokeWidth": 1, + "fillColor": "#00ff0033" + } + }, + "polygons": { + "roi": { + "color": "#0000ff" + } + } + }, + "tools": { + "rulers": [ + { + "firstPoint": [-30.5, 12.25, -12.5], + "secondPoint": [18.75, 44, -12.5], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, -12.5] + }, + "slice": 42, + "labelName": "lesion", + "name": "Ruler", + "metadata": { + "measuredBy": "reader-1" + } + } + ], + "rectangles": [ + { + "firstPoint": [-30.5, 12.25, -12.5], + "secondPoint": [18.75, 44, -12.5], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, -12.5] + }, + "slice": 42, + "labelName": "lesion", + "name": "Rectangle" + } + ], + "polygons": [ + { + "points": [ + [-20, 0, -12.5], + [10, 0, -12.5], + [10, 30, -12.5], + [-20, 30, -12.5] + ], + "frameOfReference": { + "planeNormal": [0, 0, 1], + "planeOrigin": [0, 0, -12.5] + }, + "slice": 42, + "labelName": "roi", + "name": "Polygon" + } + ] + } +} diff --git a/backend-contract/fixtures/wire/stage-input.annotations.json b/backend-contract/fixtures/wire/stage-input.annotations.json new file mode 100644 index 000000000..c9f4a296d --- /dev/null +++ b/backend-contract/fixtures/wire/stage-input.annotations.json @@ -0,0 +1,12 @@ +{ + "type": "annotations", + "name": "chest-ct.annotations.json", + "referenceImage": { + "type": "image", + "format": "dicom-series", + "uris": [ + "/api/v1/file/aaaaaaaaaaaaaaaaaaaaaaaa/proxiable/001.dcm", + "/api/v1/file/bbbbbbbbbbbbbbbbbbbbbbbb/proxiable/002.dcm" + ] + } +} diff --git a/backend-contract/generated/annotations-file.schema.json b/backend-contract/generated/annotations-file.schema.json new file mode 100644 index 000000000..1652ded68 --- /dev/null +++ b/backend-contract/generated/annotations-file.schema.json @@ -0,0 +1,422 @@ +{ + "$comment": "Structural validation only. Implement backend-contract validateAnnotationsFileSemantics after this schema: every planeNormal must be nonzero, and every nonempty labelName must be declared in its own tool-kind label namespace.", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "schemaVersion": { + "type": "number", + "const": 1 + }, + "space": { + "type": "string", + "const": "LPS" + }, + "labels": { + "type": "object", + "properties": { + "rulers": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "object", + "properties": { + "color": { + "type": "string" + }, + "strokeWidth": { + "type": "number" + }, + "fillColor": { + "type": "string" + } + }, + "additionalProperties": false + } + }, + "rectangles": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "object", + "properties": { + "color": { + "type": "string" + }, + "strokeWidth": { + "type": "number" + }, + "fillColor": { + "type": "string" + } + }, + "additionalProperties": false + } + }, + "polygons": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "object", + "properties": { + "color": { + "type": "string" + }, + "strokeWidth": { + "type": "number" + }, + "fillColor": { + "type": "string" + } + }, + "additionalProperties": false + } + } + }, + "additionalProperties": false + }, + "tools": { + "type": "object", + "properties": { + "rulers": { + "type": "array", + "items": { + "type": "object", + "properties": { + "firstPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "secondPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "frameOfReference": { + "type": "object", + "properties": { + "planeNormal": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "planeOrigin": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "required": [ + "planeNormal", + "planeOrigin" + ], + "additionalProperties": false + }, + "slice": { + "type": "number" + }, + "frame": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "labelName": { + "type": "string" + }, + "name": { + "type": "string" + }, + "metadata": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "string" + } + } + }, + "required": [ + "firstPoint", + "secondPoint", + "frameOfReference" + ], + "additionalProperties": false + } + }, + "rectangles": { + "type": "array", + "items": { + "type": "object", + "properties": { + "firstPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "secondPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "frameOfReference": { + "type": "object", + "properties": { + "planeNormal": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "planeOrigin": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "required": [ + "planeNormal", + "planeOrigin" + ], + "additionalProperties": false + }, + "slice": { + "type": "number" + }, + "frame": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "labelName": { + "type": "string" + }, + "name": { + "type": "string" + }, + "metadata": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "string" + } + } + }, + "required": [ + "firstPoint", + "secondPoint", + "frameOfReference" + ], + "additionalProperties": false + } + }, + "polygons": { + "type": "array", + "items": { + "type": "object", + "properties": { + "points": { + "minItems": 3, + "type": "array", + "items": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "frameOfReference": { + "type": "object", + "properties": { + "planeNormal": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "planeOrigin": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "required": [ + "planeNormal", + "planeOrigin" + ], + "additionalProperties": false + }, + "slice": { + "type": "number" + }, + "frame": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "labelName": { + "type": "string" + }, + "name": { + "type": "string" + }, + "metadata": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "string" + } + } + }, + "required": [ + "points", + "frameOfReference" + ], + "additionalProperties": false + } + } + }, + "additionalProperties": false + } + }, + "required": [ + "schemaVersion", + "space", + "tools" + ], + "additionalProperties": {} +} diff --git a/backend-contract/generated/openapi.json b/backend-contract/generated/openapi.json index ba37240c7..5db5e18b0 100644 --- a/backend-contract/generated/openapi.json +++ b/backend-contract/generated/openapi.json @@ -216,10 +216,10 @@ "tags": [ "context" ], - "summary": "Stage a parent-bound labelmap as a transient input; returns backend-minted URIs the client round-trips as an InputValue at submit.", + "summary": "Stage a parent-bound labelmap or annotations file as a transient input; returns backend-minted URIs the client round-trips as an InputValue at submit.", "requestBody": { "required": true, - "description": "A typed staged resource: the labelmap bytes plus its durable reference-image relationship.", + "description": "A typed staged resource: the bytes plus their durable reference-image relationship. The descriptor `type` selects which stageable resource the bytes are; an unknown type is rejected.", "content": { "multipart/form-data": { "schema": { @@ -850,47 +850,94 @@ "additionalProperties": false }, "StageInputDescriptor": { - "type": "object", - "properties": { - "type": { - "type": "string", - "const": "labelmap" - }, - "name": { - "type": "string", - "minLength": 1 + "oneOf": [ + { + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "labelmap" + }, + "name": { + "type": "string", + "minLength": 1 + }, + "referenceImage": { + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "image" + }, + "format": { + "type": "string" + }, + "uris": { + "minItems": 1, + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "type", + "uris" + ], + "additionalProperties": false + } + }, + "required": [ + "type", + "name", + "referenceImage" + ], + "additionalProperties": false }, - "referenceImage": { + { "type": "object", "properties": { "type": { "type": "string", - "const": "image" + "const": "annotations" }, - "format": { - "type": "string" + "name": { + "type": "string", + "minLength": 1 }, - "uris": { - "minItems": 1, - "type": "array", - "items": { - "type": "string" - } + "referenceImage": { + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "image" + }, + "format": { + "type": "string" + }, + "uris": { + "minItems": 1, + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "type", + "uris" + ], + "additionalProperties": false } }, "required": [ "type", - "uris" + "name", + "referenceImage" ], "additionalProperties": false } - }, - "required": [ - "type", - "name", - "referenceImage" - ], - "additionalProperties": false + ] }, "NeutralJobStatus": { "oneOf": [ @@ -1558,6 +1605,427 @@ } ] }, + "AnnotationsFile": { + "$comment": "Structural validation only. Implement backend-contract validateAnnotationsFileSemantics after this schema: every planeNormal must be nonzero, and every nonempty labelName must be declared in its own tool-kind label namespace.", + "type": "object", + "properties": { + "schemaVersion": { + "type": "number", + "const": 1 + }, + "space": { + "type": "string", + "const": "LPS" + }, + "labels": { + "type": "object", + "properties": { + "rulers": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "object", + "properties": { + "color": { + "type": "string" + }, + "strokeWidth": { + "type": "number" + }, + "fillColor": { + "type": "string" + } + }, + "additionalProperties": false + } + }, + "rectangles": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "object", + "properties": { + "color": { + "type": "string" + }, + "strokeWidth": { + "type": "number" + }, + "fillColor": { + "type": "string" + } + }, + "additionalProperties": false + } + }, + "polygons": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "object", + "properties": { + "color": { + "type": "string" + }, + "strokeWidth": { + "type": "number" + }, + "fillColor": { + "type": "string" + } + }, + "additionalProperties": false + } + } + }, + "additionalProperties": false + }, + "tools": { + "type": "object", + "properties": { + "rulers": { + "type": "array", + "items": { + "type": "object", + "properties": { + "firstPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "secondPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "frameOfReference": { + "type": "object", + "properties": { + "planeNormal": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "planeOrigin": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "required": [ + "planeNormal", + "planeOrigin" + ], + "additionalProperties": false + }, + "slice": { + "type": "number" + }, + "frame": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "labelName": { + "type": "string" + }, + "name": { + "type": "string" + }, + "metadata": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "string" + } + } + }, + "required": [ + "firstPoint", + "secondPoint", + "frameOfReference" + ], + "additionalProperties": false + } + }, + "rectangles": { + "type": "array", + "items": { + "type": "object", + "properties": { + "firstPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "secondPoint": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "frameOfReference": { + "type": "object", + "properties": { + "planeNormal": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "planeOrigin": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "required": [ + "planeNormal", + "planeOrigin" + ], + "additionalProperties": false + }, + "slice": { + "type": "number" + }, + "frame": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "labelName": { + "type": "string" + }, + "name": { + "type": "string" + }, + "metadata": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "string" + } + } + }, + "required": [ + "firstPoint", + "secondPoint", + "frameOfReference" + ], + "additionalProperties": false + } + }, + "polygons": { + "type": "array", + "items": { + "type": "object", + "properties": { + "points": { + "minItems": 3, + "type": "array", + "items": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "frameOfReference": { + "type": "object", + "properties": { + "planeNormal": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + }, + "planeOrigin": { + "minItems": 3, + "maxItems": 3, + "type": "array", + "prefixItems": [ + { + "type": "number" + }, + { + "type": "number" + }, + { + "type": "number" + } + ] + } + }, + "required": [ + "planeNormal", + "planeOrigin" + ], + "additionalProperties": false + }, + "slice": { + "type": "number" + }, + "frame": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "labelName": { + "type": "string" + }, + "name": { + "type": "string" + }, + "metadata": { + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^(?!__proto__$)" + }, + "additionalProperties": { + "type": "string" + } + } + }, + "required": [ + "points", + "frameOfReference" + ], + "additionalProperties": false + } + } + }, + "additionalProperties": false + } + }, + "required": [ + "schemaVersion", + "space", + "tools" + ], + "additionalProperties": {} + }, "TaskSummary": { "type": "object", "description": "Advisory display metadata for one task in the picker. Pass-through: the client renders it but validates only id/title; every other field is OPTIONAL advisory display metadata a backend MAY omit, and the client never dispatches on it.", diff --git a/backend-contract/generated/stage-input-descriptor.schema.json b/backend-contract/generated/stage-input-descriptor.schema.json index 59a751a27..ffa29090a 100644 --- a/backend-contract/generated/stage-input-descriptor.schema.json +++ b/backend-contract/generated/stage-input-descriptor.schema.json @@ -1,44 +1,91 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", - "properties": { - "type": { - "type": "string", - "const": "labelmap" - }, - "name": { - "type": "string", - "minLength": 1 + "oneOf": [ + { + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "labelmap" + }, + "name": { + "type": "string", + "minLength": 1 + }, + "referenceImage": { + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "image" + }, + "format": { + "type": "string" + }, + "uris": { + "minItems": 1, + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "type", + "uris" + ], + "additionalProperties": false + } + }, + "required": [ + "type", + "name", + "referenceImage" + ], + "additionalProperties": false }, - "referenceImage": { + { "type": "object", "properties": { "type": { "type": "string", - "const": "image" + "const": "annotations" }, - "format": { - "type": "string" + "name": { + "type": "string", + "minLength": 1 }, - "uris": { - "minItems": 1, - "type": "array", - "items": { - "type": "string" - } + "referenceImage": { + "type": "object", + "properties": { + "type": { + "type": "string", + "const": "image" + }, + "format": { + "type": "string" + }, + "uris": { + "minItems": 1, + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "type", + "uris" + ], + "additionalProperties": false } }, "required": [ "type", - "uris" + "name", + "referenceImage" ], "additionalProperties": false } - }, - "required": [ - "type", - "name", - "referenceImage" - ], - "additionalProperties": false + ] } diff --git a/backend-contract/processing/__tests__/annotations.spec.ts b/backend-contract/processing/__tests__/annotations.spec.ts new file mode 100644 index 000000000..0192c06c4 --- /dev/null +++ b/backend-contract/processing/__tests__/annotations.spec.ts @@ -0,0 +1,442 @@ +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; + +import { + ANNOTATIONS_FILE_SCHEMA_VERSION, + ANNOTATIONS_SPACE, + ANNOTATION_TOOL_KINDS, + annotationLabelSchema, + annotationsFileSchema, + annotationsFileStructuralSchema, + validateAnnotationsFileSemantics, + wirePolygonSchema, + wireRectangleSchema, + wireRulerSchema, +} from '../annotations'; +import { generateJsonSchemas } from '../schema-json'; +import { loadFixture } from './loadFixtures'; + +const golden = () => loadFixture('wire/annotations-file.json'); + +// Parse once so the fixture-derived clones below have a typed object source. +const goldenParsed = () => annotationsFileSchema.parse(golden()); + +// --------------------------------------------------------------------------- +// The golden interchange example +// --------------------------------------------------------------------------- + +describe('the golden annotations interchange file', () => { + it('pins the fail-closed envelope constants', () => { + expect(ANNOTATIONS_FILE_SCHEMA_VERSION).toBe(1); + expect(ANNOTATIONS_SPACE).toBe('LPS'); + expect([...ANNOTATION_TOOL_KINDS]).toEqual([ + 'rulers', + 'rectangles', + 'polygons', + ]); + }); + + it('validates and carries one of each tool kind', () => { + const file = goldenParsed(); + expect(file.schemaVersion).toBe(1); + expect(file.space).toBe('LPS'); + expect(file.tools.rulers).toHaveLength(1); + expect(file.tools.rectangles).toHaveLength(1); + expect(file.tools.polygons).toHaveLength(1); + }); + + it('keeps the SAME label name independent per tool kind, with its own style', () => { + // The whole reason the label namespaces are per tool kind: the three client + // stores are independent and may legally style `lesion` differently. + const file = goldenParsed(); + expect(file.tools.rulers?.[0].labelName).toBe('lesion'); + expect(file.tools.rectangles?.[0].labelName).toBe('lesion'); + expect(file.labels?.rulers?.lesion.color).toBe('#ff0000'); + expect(file.labels?.rectangles?.lesion.color).toBe('#00ff00'); + expect(file.labels?.rulers?.lesion).not.toHaveProperty('fillColor'); + expect(file.labels?.rectangles?.lesion.fillColor).toBe('#00ff0033'); + }); + + it('carries advisory per-tool metadata', () => { + const file = goldenParsed(); + expect(file.tools.rulers?.[0].metadata).toEqual({ + measuredBy: 'reader-1', + }); + }); + + it('has no semantic issues', () => { + expect(validateAnnotationsFileSemantics(golden())).toEqual([]); + }); +}); + +// --------------------------------------------------------------------------- +// Fail-closed envelope +// --------------------------------------------------------------------------- + +describe('the envelope fails closed', () => { + it.each([ + ['a future schemaVersion', 'negative/annotations-bad-schema-version.json'], + ['a non-LPS space', 'negative/annotations-bad-space.json'], + ])('rejects %s', (_label, path) => { + expect(annotationsFileSchema.safeParse(loadFixture(path)).success).toBe( + false + ); + }); + + it('requires the tools envelope', () => { + expect( + annotationsFileSchema.safeParse({ schemaVersion: 1, space: 'LPS' }) + .success + ).toBe(false); + }); + + it('accepts an empty tools envelope (a valid, empty result)', () => { + const file = annotationsFileSchema.parse({ + schemaVersion: 1, + space: 'LPS', + tools: {}, + }); + expect(file.tools.rulers).toBeUndefined(); + }); + + it('preserves an unrecognized producer field on the envelope', () => { + const file = annotationsFileSchema.parse({ + ...goldenParsed(), + producerHint: 'keep-me', + }) as Record; + expect(file.producerHint).toBe('keep-me'); + }); + + it('rejects a false labels envelope rather than treating it as absent', () => { + expect( + annotationsFileSchema.safeParse({ + schemaVersion: 1, + space: 'LPS', + labels: false, + tools: {}, + }).success + ).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// Strict per-tool records — no session state on the wire +// --------------------------------------------------------------------------- + +describe('per-tool records are strict', () => { + const ruler = () => ({ + firstPoint: [0, 0, 0], + secondPoint: [1, 1, 0], + frameOfReference: { planeNormal: [0, 0, 1], planeOrigin: [0, 0, 0] }, + }); + + it.each([ + 'id', + 'imageID', + 'color', + 'strokeWidth', + 'fillColor', + 'hidden', + 'placing', + 'source', + 'label', + ])('rejects the session-only field %s', (field) => { + expect( + wireRulerSchema.safeParse({ ...ruler(), [field]: 'x' }).success + ).toBe(false); + }); + + it('rejects a forbidden session field inside the whole file', () => { + const bad = loadFixture('negative/annotations-session-field.json'); + expect(annotationsFileSchema.safeParse(bad).success).toBe(false); + }); + + it('requires a frame of reference', () => { + const { frameOfReference, ...noFrame } = ruler(); + void frameOfReference; + expect(wireRulerSchema.safeParse(noFrame).success).toBe(false); + }); + + it('accepts a scaled nonzero plane normal', () => { + expect( + wireRulerSchema.safeParse({ + ...ruler(), + frameOfReference: { + planeNormal: [0, 0, 2], + planeOrigin: [0, 0, 0], + }, + }).success + ).toBe(true); + }); + + it('rejects an extra frame-of-reference field', () => { + expect( + wireRulerSchema.safeParse({ + ...ruler(), + frameOfReference: { + ...ruler().frameOfReference, + coordinateSystem: 'LPS', + }, + }).success + ).toBe(false); + }); + + it('requires three-component points', () => { + expect( + wireRulerSchema.safeParse({ ...ruler(), secondPoint: [1, 1] }).success + ).toBe(false); + }); + + it('accepts opposite corners for an image-axis-aligned rectangle', () => { + expect(wireRectangleSchema.safeParse(ruler()).success).toBe(true); + }); + + it('rejects a polygon with fewer than three points', () => { + const bad = loadFixture('negative/annotations-two-point-polygon.json'); + expect(annotationsFileSchema.safeParse(bad).success).toBe(false); + expect( + wirePolygonSchema.safeParse({ + points: [ + [0, 0, 0], + [1, 0, 0], + ], + frameOfReference: { planeNormal: [0, 0, 1], planeOrigin: [0, 0, 0] }, + }).success + ).toBe(false); + }); + + it('accepts the advisory slice/frame echoes', () => { + expect( + wireRulerSchema.safeParse({ ...ruler(), slice: 42, frame: 3 }).success + ).toBe(true); + }); + + it('requires finite coordinates', () => { + expect( + wireRulerSchema.safeParse({ ...ruler(), firstPoint: [Infinity, 0, 0] }) + .success + ).toBe(false); + expect( + wireRulerSchema.safeParse({ ...ruler(), secondPoint: [0, NaN, 0] }) + .success + ).toBe(false); + }); + + // A frame indexes a cine loop: only a non-negative integer is honorable. + it.each([1.5, -1, Infinity, NaN, Number.MAX_SAFE_INTEGER + 1])( + 'rejects frame %s', + (frame) => { + expect(wireRulerSchema.safeParse({ ...ruler(), frame }).success).toBe( + false + ); + } + ); + + it('rejects an explicit null labelName', () => { + expect( + wireRulerSchema.safeParse({ ...ruler(), labelName: null }).success + ).toBe(false); + }); + + it('rejects a non-finite slice echo', () => { + expect( + wireRulerSchema.safeParse({ ...ruler(), slice: Infinity }).success + ).toBe(false); + }); + + it.each([ + ['a non-object body', false], + ['an unknown style field', { opacity: 0.5 }], + ['a non-string color', { color: 123 }], + ['a non-number stroke width', { strokeWidth: '2' }], + ])('rejects a label with %s', (_case, label) => { + expect(annotationLabelSchema.safeParse(label).success).toBe(false); + }); + + it('reserves __proto__ in label and metadata records', () => { + const unsafeLabels = JSON.parse('{"__proto__": {}}'); + const unsafeMetadata = JSON.parse('{"__proto__": "value"}'); + expect( + annotationsFileSchema.safeParse({ + schemaVersion: 1, + space: 'LPS', + labels: { rulers: unsafeLabels }, + tools: {}, + }).success + ).toBe(false); + expect( + annotationsFileSchema.safeParse({ + schemaVersion: 1, + space: 'LPS', + tools: { + rulers: [{ ...ruler(), metadata: unsafeMetadata }], + }, + }).success + ).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// Label-reference integrity — the semantic pass +// --------------------------------------------------------------------------- + +describe('label references are namespaced and must resolve', () => { + it('rejects a zero plane normal through the semantic pass', () => { + const bad = loadFixture('negative/annotations-zero-normal.json'); + expect(annotationsFileStructuralSchema.safeParse(bad).success).toBe(true); + expect(annotationsFileSchema.safeParse(bad).success).toBe(false); + expect(validateAnnotationsFileSemantics(bad)).toEqual([ + { + message: 'planeNormal must be a nonzero vector', + path: ['tools', 'rulers', 0, 'frameOfReference', 'planeNormal'], + }, + ]); + }); + + it('rejects a labelName declared only in ANOTHER tool kind', () => { + const bad = loadFixture('negative/annotations-dangling-label.json'); + // Structurally fine: only the semantic pass can see the dangling reference. + expect(annotationsFileStructuralSchema.safeParse(bad).success).toBe(true); + expect(annotationsFileSchema.safeParse(bad).success).toBe(false); + expect(validateAnnotationsFileSemantics(bad)).toEqual([ + { + message: 'labelName lesion is not declared in labels.rulers', + path: ['tools', 'rulers', 0, 'labelName'], + }, + ]); + }); + + it('rejects any labelName when the file declares no labels at all', () => { + expect( + annotationsFileSchema.safeParse({ + schemaVersion: 1, + space: 'LPS', + tools: { + rulers: [ + { + firstPoint: [0, 0, 0], + secondPoint: [1, 1, 0], + frameOfReference: { + planeNormal: [0, 0, 1], + planeOrigin: [0, 0, 0], + }, + labelName: 'lesion', + }, + ], + }, + }).success + ).toBe(false); + }); + + it('accepts an unlabeled tool (labelName omitted)', () => { + expect( + annotationsFileSchema.safeParse({ + schemaVersion: 1, + space: 'LPS', + tools: { + rulers: [ + { + firstPoint: [0, 0, 0], + secondPoint: [1, 1, 0], + frameOfReference: { + planeNormal: [0, 0, 1], + planeOrigin: [0, 0, 0], + }, + }, + ], + }, + }).success + ).toBe(true); + }); + + it('reports one issue per dangling reference, addressed by path', () => { + const file = goldenParsed(); + file.tools.polygons![0].labelName = 'not-declared'; + expect(validateAnnotationsFileSemantics(file)).toEqual([ + { + message: 'labelName not-declared is not declared in labels.polygons', + path: ['tools', 'polygons', 0, 'labelName'], + }, + ]); + }); + + it('returns no issues for a non-object payload (structural pass owns that)', () => { + expect(validateAnnotationsFileSemantics(null)).toEqual([]); + expect(validateAnnotationsFileSemantics('nope')).toEqual([]); + }); +}); + +// --------------------------------------------------------------------------- +// One definition, two validators +// --------------------------------------------------------------------------- + +describe('the generated annotations-file schema agrees with the zod source', () => { + const generated = generateJsonSchemas()['annotations-file']; + + it('names the required semantic pass in the artifact itself', () => { + expect((generated as { $comment?: string }).$comment).toContain( + 'validateAnnotationsFileSemantics' + ); + }); + + it('structurally accepts the golden fixture', () => { + const structural = z.fromJSONSchema(generated); + expect(structural.safeParse(golden()).success).toBe(true); + }); + + it('leaves the zero-normal rule to the named semantic pass', () => { + const structural = z.fromJSONSchema(generated); + expect( + structural.safeParse(loadFixture('negative/annotations-zero-normal.json')) + .success + ).toBe(true); + }); + + it('structurally rejects the envelope and shape negatives', () => { + const structural = z.fromJSONSchema(generated); + [ + 'negative/annotations-bad-schema-version.json', + 'negative/annotations-bad-space.json', + 'negative/annotations-two-point-polygon.json', + 'negative/annotations-session-field.json', + ].forEach((path) => { + expect( + structural.safeParse(loadFixture(path)).success, + `expected ${path} to be rejected structurally` + ).toBe(false); + }); + }); + + it('closes the point tuples to exactly three components', () => { + const structural = z.fromJSONSchema(generated); + const file = goldenParsed(); + file.tools.rulers![0].secondPoint = [1, 1, 0, 1] as never; + expect(structural.safeParse(file).success).toBe(false); + }); + + it('carries the reserved record-key rule into JSON Schema', () => { + const at = (path: string[]) => + path.reduce( + (node, key) => node[key] as Record, + generated as Record + ); + ANNOTATION_TOOL_KINDS.forEach((kind) => { + [ + ['properties', 'labels', 'properties', kind], + [ + 'properties', + 'tools', + 'properties', + kind, + 'items', + 'properties', + 'metadata', + ], + ].forEach((path) => { + expect(at(path).propertyNames).toMatchObject({ + pattern: '^(?!__proto__$)', + }); + }); + }); + }); +}); diff --git a/backend-contract/processing/__tests__/wire.spec.ts b/backend-contract/processing/__tests__/wire.spec.ts index 2bb3d87d4..2ee69a069 100644 --- a/backend-contract/processing/__tests__/wire.spec.ts +++ b/backend-contract/processing/__tests__/wire.spec.ts @@ -4,6 +4,7 @@ import { JOB_STATES, RESULT_INTENTS, INTENT_VOCABULARY_VERSION, + STAGEABLE_TYPES, inputValueSchema, stageInputDescriptorSchema, neutralJobStatusSchema, @@ -67,6 +68,46 @@ describe('staged resource descriptor fixtures', () => { expect(descriptor.referenceImage.uris).toHaveLength(2); }); + it('binds staged annotations bytes to a durable reference image', () => { + const descriptor = stageInputDescriptorSchema.parse( + wire['stage-input.annotations'] + ); + expect(descriptor.type).toBe('annotations'); + expect(descriptor.name).toBe('chest-ct.annotations.json'); + expect(descriptor.referenceImage.type).toBe('image'); + expect(descriptor.referenceImage.uris).toHaveLength(2); + }); + + it('pins the stageable types and their identical key set', () => { + // A new stageable type adds a union member and nothing else: the descriptor + // key set is the same for every type, so the backend runs ONE code path. + expect([...STAGEABLE_TYPES]).toEqual(['labelmap', 'annotations']); + const keys = (name: string) => + Object.keys( + stageInputDescriptorSchema.parse(wire[name]) as Record + ).sort(); + expect(keys('stage-input.annotations')).toEqual( + keys('stage-input.labelmap') + ); + }); + + it('rejects an unknown staged resource type (staging is fail-closed)', () => { + const unknownType = loadFixture('negative/stage-input-unknown-type.json'); + expect(stageInputDescriptorSchema.safeParse(unknownType).success).toBe( + false + ); + }); + + it('rejects an annotations descriptor without reference provenance', () => { + expect( + stageInputDescriptorSchema.safeParse({ + type: 'annotations', + name: 'rois.annotations.json', + referenceImage: { type: 'image', uris: [] }, + }).success + ).toBe(false); + }); + it('rejects a labelmap descriptor without reference provenance', () => { expect( stageInputDescriptorSchema.safeParse({ diff --git a/backend-contract/processing/annotations.ts b/backend-contract/processing/annotations.ts new file mode 100644 index 000000000..f846a1d8a --- /dev/null +++ b/backend-contract/processing/annotations.ts @@ -0,0 +1,242 @@ +// Vector annotations staged into a task or returned as a result. Coordinates +// are world LPS millimeters. Tool records exclude session identity and state; +// labels are namespaced by tool kind because the client stores are independent. +// Unknown envelope fields survive round-trip without gaining behavior. + +import { z } from 'zod'; + +// The integer file version. Bump only on a shape change; new optional fields +// do not need it. +export const ANNOTATIONS_FILE_SCHEMA_VERSION = 1; + +// World millimeters, LPS. The only space this format speaks. +export const ANNOTATIONS_SPACE = 'LPS' as const; + +// The file-name extension a task spec and a backend both match these bytes on. +export const ANNOTATIONS_FILE_EXTENSION = '.annotations.json'; + +// The three tool kinds, and the key set of both `tools` and `labels`. +export const ANNOTATION_TOOL_KINDS = [ + 'rulers', + 'rectangles', + 'polygons', +] as const; +export type AnnotationToolKind = (typeof ANNOTATION_TOOL_KINDS)[number]; + +// --------------------------------------------------------------------------- +// Geometry +// --------------------------------------------------------------------------- + +// Finite: JSON cannot carry NaN, and an Infinity coordinate cannot be placed. +const finiteNumber = z.number().finite(); +const vector3Schema = z.tuple([finiteNumber, finiteNumber, finiteNumber]); +// `__proto__` is not a portable record key: Zod/object construction can treat +// it differently from JSON Schema and Python dictionaries. Reserve it rather +// than let one validator silently drop a label or metadata entry. +export const ANNOTATIONS_RESERVED_RECORD_KEY = '__proto__'; +const wireRecordKeySchema = z.string(); + +// The 2D plane an annotation is drawn on, in world LPS mm. `planeNormal` must +// be nonzero, but its magnitude carries no information: consumers normalize it +// before placement. A consumer resolves the plane against the referenced +// image's own metadata; a producer that cannot author a plane should echo the +// frame of an input annotation. +export const annotationFrameOfReferenceSchema = z.strictObject({ + planeNormal: vector3Schema, + planeOrigin: vector3Schema, +}); +export type AnnotationFrameOfReference = z.infer< + typeof annotationFrameOfReferenceSchema +>; + +// Fields every tool kind carries. `frameOfReference` is the required locator. +// `slice`/`frame` are ADVISORY echoes of where the producer saw the annotation: +// a consumer re-derives the slice from `frameOfReference` against its own image +// and must never trust these to place a tool. +const toolCore = { + frameOfReference: annotationFrameOfReferenceSchema, + slice: finiteNumber.optional(), + // A frame indexes a cine loop, so only a non-negative integer can ever be + // honored; anything else would render the tool unreachable. + frame: z.number().int().nonnegative().optional(), + labelName: z.string().optional(), + name: z.string().optional(), + metadata: z.record(wireRecordKeySchema, z.string()).optional(), +}; + +// Session identity, placement state, provenance, and inline style are rejected. +export const wireRulerSchema = z.strictObject({ + firstPoint: vector3Schema, + secondPoint: vector3Schema, + ...toolCore, +}); +export type WireRuler = z.infer; + +// Rectangle points are opposite corners; edges follow the referenced image's +// in-plane axes. Use a polygon for a rotated box. +export const wireRectangleSchema = wireRulerSchema; +export type WireRectangle = z.infer; + +export const wirePolygonSchema = z.strictObject({ + points: z.array(vector3Schema).min(3), + ...toolCore, +}); +export type WirePolygon = z.infer; + +// --------------------------------------------------------------------------- +// Labels +// --------------------------------------------------------------------------- + +// A label's style. Every field is optional: a label may exist purely as a name. +export const annotationLabelSchema = z.strictObject({ + color: z.string().optional(), + strokeWidth: z.number().optional(), + fillColor: z.string().optional(), +}); +export type AnnotationLabel = z.infer; + +// One label namespace per tool kind, each keyed by `labelName` — cross-boundary +// label identity is the NAME, never an id. +const labelsByKindSchema = z.strictObject({ + rulers: z.record(wireRecordKeySchema, annotationLabelSchema).optional(), + rectangles: z.record(wireRecordKeySchema, annotationLabelSchema).optional(), + polygons: z.record(wireRecordKeySchema, annotationLabelSchema).optional(), +}); +export type AnnotationLabelsByKind = z.infer; + +// --------------------------------------------------------------------------- +// The file +// --------------------------------------------------------------------------- + +export const annotationsFileStructuralSchema = z + .object({ + schemaVersion: z.literal(ANNOTATIONS_FILE_SCHEMA_VERSION), + space: z.literal(ANNOTATIONS_SPACE), + labels: labelsByKindSchema.optional(), + tools: z.object({ + rulers: z.array(wireRulerSchema).optional(), + rectangles: z.array(wireRectangleSchema).optional(), + polygons: z.array(wirePolygonSchema).optional(), + }), + }) + .passthrough(); + +export type AnnotationsSemanticIssue = { + message: string; + path: (string | number)[]; +}; + +const isRecord = (value: unknown): value is Record => + value !== null && typeof value === 'object'; + +const hasReservedRecordKey = (value: unknown): boolean => + isRecord(value) && + Object.prototype.hasOwnProperty.call(value, ANNOTATIONS_RESERVED_RECORD_KEY); + +const validateReservedRecordKeys = ( + file: unknown +): AnnotationsSemanticIssue[] => { + if (!isRecord(file)) return []; + const issues: AnnotationsSemanticIssue[] = []; + const { labels, tools } = file; + if (isRecord(labels)) { + ANNOTATION_TOOL_KINDS.forEach((kind) => { + if (hasReservedRecordKey(labels[kind])) { + issues.push({ + message: `${ANNOTATIONS_RESERVED_RECORD_KEY} is a reserved label name`, + path: ['labels', kind, ANNOTATIONS_RESERVED_RECORD_KEY], + }); + } + }); + } + + if (isRecord(tools)) { + ANNOTATION_TOOL_KINDS.forEach((kind) => { + const entries = tools[kind]; + if (!Array.isArray(entries)) return; + entries.forEach((entry, index) => { + if (isRecord(entry) && hasReservedRecordKey(entry.metadata)) { + issues.push({ + message: `${ANNOTATIONS_RESERVED_RECORD_KEY} is a reserved metadata key`, + path: [ + 'tools', + kind, + index, + 'metadata', + ANNOTATIONS_RESERVED_RECORD_KEY, + ], + }); + } + }); + }); + } + return issues; +}; + +// JSON Schema cannot express "this string key exists in that sibling map", so +// label-reference integrity is a semantic pass a backend MUST run after +// structural validation — the same two-pass shape as the task spec. The +// normative zod schema below calls this same implementation, so the two +// validation paths cannot drift. +export const validateAnnotationsFileSemantics = ( + file: unknown +): AnnotationsSemanticIssue[] => { + const issues = validateReservedRecordKeys(file); + if (!isRecord(file) || !isRecord(file.tools)) return issues; + const tools = file.tools; + const labels = isRecord(file.labels) ? file.labels : {}; + + ANNOTATION_TOOL_KINDS.forEach((kind) => { + const entries = (tools as Record)[kind]; + if (!Array.isArray(entries)) return; + const namespace = labels[kind]; + entries.forEach((entry, index) => { + if (!isRecord(entry)) return; + const { frameOfReference, labelName } = entry; + const planeNormal = isRecord(frameOfReference) + ? frameOfReference.planeNormal + : undefined; + if ( + Array.isArray(planeNormal) && + planeNormal.length === 3 && + planeNormal.every( + (component) => typeof component === 'number' && component === 0 + ) + ) { + issues.push({ + message: 'planeNormal must be a nonzero vector', + path: ['tools', kind, index, 'frameOfReference', 'planeNormal'], + }); + } + if (typeof labelName !== 'string' || labelName === '') return; + const declared = + isRecord(namespace) && + Object.prototype.hasOwnProperty.call(namespace, labelName); + if (declared) return; + issues.push({ + message: `labelName ${labelName} is not declared in labels.${kind}`, + path: ['tools', kind, index, 'labelName'], + }); + }); + }); + return issues; +}; + +const annotationsFileSchemaAfterStructural = + annotationsFileStructuralSchema.superRefine((file, ctx) => { + validateAnnotationsFileSemantics(file).forEach((issue) => + ctx.addIssue({ code: 'custom', ...issue }) + ); + }); + +// Zod materializes records before `superRefine`, and `__proto__` can disappear +// during that construction. Inspect this one reserved key on the raw input; +// all other semantic checks run after structural parsing as usual. +export const annotationsFileSchema = z.preprocess((file, ctx) => { + validateReservedRecordKeys(file).forEach((issue) => + ctx.addIssue({ code: 'custom', input: file, ...issue }) + ); + return file; +}, annotationsFileSchemaAfterStructural); + +export type AnnotationsFile = z.infer; diff --git a/backend-contract/processing/index.ts b/backend-contract/processing/index.ts index 07ce6afe6..0b31a0183 100644 --- a/backend-contract/processing/index.ts +++ b/backend-contract/processing/index.ts @@ -12,4 +12,5 @@ export * from './task-spec'; export * from './wire'; +export * from './annotations'; export * from './ids'; diff --git a/backend-contract/processing/openapi.ts b/backend-contract/processing/openapi.ts index 4d9d0a92b..21b5d31e1 100644 --- a/backend-contract/processing/openapi.ts +++ b/backend-contract/processing/openapi.ts @@ -43,6 +43,10 @@ const WIRE_COMPONENTS: Record = { 'job-history-detail': 'JobHistoryDetail', 'job-results': 'JobResults', 'job-results-error': 'JobResultsError', + // Not a request/response body: the interchange FILE format a staged + // `annotations` input carries. Published as a component so a backend author + // sees the whole vocabulary, including the file formats, in one document. + 'annotations-file': 'AnnotationsFile', }; // The generated schemas carry a per-schema `$schema` dialect marker; OpenAPI 3.1 @@ -327,13 +331,15 @@ const paths = (): Record => ({ operationId: 'stageInput', tags: ['context'], summary: - 'Stage a parent-bound labelmap as a transient input; returns ' + - 'backend-minted URIs the client round-trips as an InputValue at submit.', + 'Stage a parent-bound labelmap or annotations file as a transient ' + + 'input; returns backend-minted URIs the client round-trips as an ' + + 'InputValue at submit.', requestBody: { required: true, description: - 'A typed staged resource: the labelmap bytes plus its durable ' + - 'reference-image relationship.', + 'A typed staged resource: the bytes plus their durable ' + + 'reference-image relationship. The descriptor `type` selects which ' + + 'stageable resource the bytes are; an unknown type is rejected.', content: { 'multipart/form-data': { schema: { diff --git a/backend-contract/processing/schema-json.ts b/backend-contract/processing/schema-json.ts index 73295dee2..2765267b9 100644 --- a/backend-contract/processing/schema-json.ts +++ b/backend-contract/processing/schema-json.ts @@ -17,6 +17,11 @@ import { z } from 'zod'; import { taskSpecSchema } from './task-spec'; +import { + ANNOTATIONS_RESERVED_RECORD_KEY, + ANNOTATION_TOOL_KINDS, + annotationsFileSchema, +} from './annotations'; import { inputValueSchema, stageInputDescriptorSchema, @@ -40,6 +45,7 @@ const schemas = { 'job-history-detail': jobHistoryDetailSchema, 'job-results': jobResultsSchema, 'job-results-error': jobResultsErrorSchema, + 'annotations-file': annotationsFileSchema, } as const; export type GeneratedSchemaName = keyof typeof schemas; @@ -67,23 +73,67 @@ const closeTupleLengths = (node: unknown): unknown => { }; }; +const asRecord = (value: unknown): Record => + value as Record; + +const descend = ( + root: Record, + path: readonly string[] +): Record => + path.reduce((node, key) => asRecord(node[key]), root); + +// Zod cannot preserve `__proto__` long enough for record-key validation, so +// the runtime schema checks the raw input. Carry the same rule explicitly in +// the generated structural artifact. +const reserveAnnotationsRecordKey = (schema: JsonSchema): JsonSchema => { + ANNOTATION_TOOL_KINDS.forEach((kind) => { + const records = [ + descend(asRecord(schema), ['properties', 'labels', 'properties', kind]), + descend(asRecord(schema), [ + 'properties', + 'tools', + 'properties', + kind, + 'items', + 'properties', + 'metadata', + ]), + ]; + records.forEach((record) => { + record.propertyNames = { + ...asRecord(record.propertyNames), + pattern: `^(?!${ANNOTATIONS_RESERVED_RECORD_KEY}$)`, + }; + }); + }); + return schema; +}; + +// Schemas whose zod source carries cross-field rules JSON Schema cannot state. +// Each names the semantic pass a backend MUST run after structural validation, +// in the generated artifact itself so the obligation travels with the schema. +const SEMANTIC_PASS_COMMENTS: Partial> = { + 'task-spec': + 'Structural validation only. Implement backend-contract validateTaskSpecSemantics after this schema and reject every fixtures/negative payload.', + 'annotations-file': + 'Structural validation only. Implement backend-contract validateAnnotationsFileSemantics after this schema: every planeNormal must be nonzero, and every nonempty labelName must be declared in its own tool-kind label namespace.', +}; + export const generateJsonSchemas = (): Record< GeneratedSchemaName, JsonSchema > => Object.fromEntries( - Object.entries(schemas).map(([name, schema]) => [ - name, - name === 'task-spec' - ? { - $comment: - 'Structural validation only. Implement backend-contract validateTaskSpecSemantics after this schema and reject every fixtures/negative payload.', - ...(closeTupleLengths( - z.toJSONSchema(schema, { unrepresentable: 'any' }) - ) as JsonSchema), - } - : closeTupleLengths(z.toJSONSchema(schema, { unrepresentable: 'any' })), - ]) + Object.entries(schemas).map(([name, schema]) => { + let structural = closeTupleLengths( + z.toJSONSchema(schema, { unrepresentable: 'any' }) + ) as JsonSchema; + if (name === 'annotations-file') { + structural = reserveAnnotationsRecordKey(structural); + } + const $comment = SEMANTIC_PASS_COMMENTS[name as GeneratedSchemaName]; + return [name, $comment ? { $comment, ...structural } : structural]; + }) ) as Record; export const GENERATED_SCHEMA_NAMES = Object.keys( diff --git a/backend-contract/processing/task-spec.ts b/backend-contract/processing/task-spec.ts index b72d1aa88..fa6d986ad 100644 --- a/backend-contract/processing/task-spec.ts +++ b/backend-contract/processing/task-spec.ts @@ -25,11 +25,15 @@ export const SPEC_VERSION = 1; // Semantic type tag. OPEN vocabulary (no closed server enum): the // tag says what an input/output IS to the task, not its byte format. Unknown -// tags are ACCEPTED, never rejected — a `z.string()`, not a `z.enum`. v1 seed -// vocabulary is `image | labelmap`; modality refinements (`ct`, `pet`) extend -// it when a task needs them. +// tags are ACCEPTED, never rejected — a `z.string()`, not a `z.enum`. The seed +// vocabulary is `image | labelmap | annotations`; modality refinements (`ct`, +// `pet`) extend it when a task needs them. Adding a tag needs no schema change +// — these constants exist so the tags are greppable, not enumerable. export const TYPE_TAG_IMAGE = 'image'; export const TYPE_TAG_LABELMAP = 'labelmap'; +// Vector annotations (rulers, rectangles, polygons) as the `annotations.ts` +// interchange file. +export const TYPE_TAG_ANNOTATIONS = 'annotations'; export const typeTagSchema = z.string(); const identifierSchema = z.string().regex(/\S/, 'id must not be empty'); diff --git a/backend-contract/processing/wire.ts b/backend-contract/processing/wire.ts index 7f6632e2d..1eacb91d8 100644 --- a/backend-contract/processing/wire.ts +++ b/backend-contract/processing/wire.ts @@ -11,7 +11,11 @@ // --------------------------------------------------------------------------- import { z } from 'zod'; -import { typeTagSchema } from './task-spec'; +import { + typeTagSchema, + TYPE_TAG_ANNOTATIONS, + TYPE_TAG_LABELMAP, +} from './task-spec'; import { pathSegmentIdSchema } from './ids'; // Bump when the intent vocabulary's shape changes so producers and the applier @@ -36,21 +40,53 @@ export const inputValueSchema = z.object({ export type InputValue = z.infer; // The typed descriptor that accompanies staged bytes. Staging creates a -// labelmap resource bound to the image it overlays; the image value is the -// same neutral, opaque-provenance shape used by ordinary task inputs. -export const stageInputDescriptorSchema = z.strictObject({ - type: z.literal('labelmap'), +// resource bound to the image it overlays; the image value is the same neutral, +// opaque-provenance shape used by ordinary task inputs. +// +// `referenceImage` stays REQUIRED for every stageable type: it is the access- +// control linkage, the durable-reference check, and the one code path the +// backend runs — a staged resource with no parent image has no owner. +const stagedReferenceImageSchema = inputValueSchema + .extend({ + type: z.literal('image'), + uris: z.array(z.string()).min(1), + }) + .strict(); + +const stagedDescriptorCommon = { name: z.string().min(1), - referenceImage: inputValueSchema - .extend({ - type: z.literal('image'), - uris: z.array(z.string()).min(1), - }) - .strict(), + referenceImage: stagedReferenceImageSchema, +}; + +// A parent-bound labelmap: the segment-group bytes overlaying the image. +const stageLabelmapDescriptorSchema = z.strictObject({ + type: z.literal(TYPE_TAG_LABELMAP), + ...stagedDescriptorCommon, }); +// A parent-bound annotations file: the vector annotations (rulers, rectangles, +// polygons) drawn on the image, as the `annotations.ts` interchange format. +const stageAnnotationsDescriptorSchema = z.strictObject({ + type: z.literal(TYPE_TAG_ANNOTATIONS), + ...stagedDescriptorCommon, +}); + +// The stageable types, discriminated by `type` over an IDENTICAL key set — a +// new stageable type adds a member here and nothing else. An unknown `type` is +// rejected: staging is fail-closed, unlike the open input-value type tag. +export const stageInputDescriptorSchema = z.discriminatedUnion('type', [ + stageLabelmapDescriptorSchema, + stageAnnotationsDescriptorSchema, +]); + export type StageInputDescriptor = z.infer; +// Read off the union above rather than restated, so the list a backend +// enumerates and the schema it validates against cannot drift apart. +export type StageableType = StageInputDescriptor['type']; +export const STAGEABLE_TYPES: readonly StageableType[] = + stageInputDescriptorSchema.options.map((option) => option.shape.type.value); + // --------------------------------------------------------------------------- // Neutral job status // --------------------------------------------------------------------------- @@ -188,7 +224,7 @@ const addSegmentGroup = z }) .passthrough(); -// The STRICT half of the vocabulary: exactly the v1 state directives, each with its +// The STRICT half of the vocabulary: every declared state directive with its // declared shape. Exported so the single applier can gate on which union member // strictly matched — a name-known-but-shape-invalid result (e.g. a broken // `segments`) carries no state directive rather than being applied as valid. From 759a397acc72a652ecfe6d7df18354f846c3cd54 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 27 Jul 2026 20:35:26 -0400 Subject: [PATCH 2/4] feat(processing): stage vector annotations as job inputs Bind every finished ruler, rectangle, and polygon on the active image as one staged file. Snapshot and validate the payload before uploads, and enforce reference-image requirements. Exclude staged values when reconstructing a completed job parent. --- src/processing/__tests__/store.spec.ts | 75 +++ src/processing/annotationKinds.ts | 16 + src/processing/components/JobsModule.vue | 216 ++++++- .../components/__tests__/JobsModule.spec.ts | 79 +++ .../components/widgets/FileWidget.vue | 60 +- .../engine/__tests__/annotationsWire.spec.ts | 530 ++++++++++++++++++ .../engine/__tests__/mintAnnotations.spec.ts | 168 ++++++ .../engine/__tests__/sourceRefs.spec.ts | 228 ++++++++ src/processing/engine/annotationsWire.ts | 298 ++++++++++ src/processing/engine/mintAnnotations.ts | 74 +++ src/processing/engine/mintInput.ts | 46 +- src/processing/engine/mintLabelmap.ts | 15 +- src/processing/engine/sourceRefs.ts | 83 ++- src/processing/store.ts | 16 +- src/types/polygon.ts | 2 +- src/types/ruler.ts | 4 +- src/utils/bugReport.ts | 3 +- src/utils/index.ts | 11 +- src/utils/path.ts | 22 + 19 files changed, 1842 insertions(+), 104 deletions(-) create mode 100644 src/processing/annotationKinds.ts create mode 100644 src/processing/engine/__tests__/annotationsWire.spec.ts create mode 100644 src/processing/engine/__tests__/mintAnnotations.spec.ts create mode 100644 src/processing/engine/annotationsWire.ts create mode 100644 src/processing/engine/mintAnnotations.ts diff --git a/src/processing/__tests__/store.spec.ts b/src/processing/__tests__/store.spec.ts index 81f9dc461..c2fd327de 100644 --- a/src/processing/__tests__/store.spec.ts +++ b/src/processing/__tests__/store.spec.ts @@ -1356,6 +1356,47 @@ describe('Providers store — re-discovered job history: slim observability adop ); }); + it.each(['labelmap', 'annotations'] as const)( + 'ignores a staged %s input when reconstructing the parent image', + async (stagedType) => { + const getJob = vi + .fn() + .mockResolvedValueOnce(jobStatus('jr', 'running')) + .mockResolvedValueOnce(jobStatus('jr', 'success')); + const provider = makeProvider({ + listJobHistory: vi.fn().mockResolvedValue({ + jobs: [handle({ state: 'running', finishedAt: undefined })], + nextCursor: null, + }), + getJob, + getJobHistoryDetail: vi.fn().mockResolvedValue({ + jobId: 'jr', + log: [], + // Staged inputs derive FROM the scene, so they are not parent + // candidates: counting one would make this pair ambiguous and the + // result would open as a top-level dataset instead of attaching. + parameters: { + inputVolume: { type: 'image', uris: ['/f/a'] }, + inputTools: { type: stagedType, uris: ['/f/staged'] }, + }, + }), + getResults: vi.fn().mockResolvedValue(resultsBundle(sampleResults)), + }); + const store = arrange(provider); + const listener = vi.fn(); + store.onJobComplete(listener); + + await store.adoptJobHistory(); + await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS); + + expect(store.submittedContexts.get(keyFor('jr'))?.activeDatasetId).toBe( + 'ds1' + ); + // The reloaded image is what the results attach to. + expect(listener.mock.calls[0][0].context?.activeDatasetId).toBe('ds1'); + } + ); + it('reconstructs a parent when its provenance contains duplicate URIs', async () => { const provider = makeProvider({ listJobHistory: vi.fn().mockResolvedValue({ @@ -1450,6 +1491,40 @@ describe('Providers store — re-discovered job history: slim observability adop expect(completion.context?.activeDatasetId).toBeUndefined(); }); + it('an annotations-only job has no reconstructible parent', async () => { + // The staged annotations value is excluded from parent candidates, so a + // task without an image input leaves nothing to re-identify. The form + // binder blocks this shape at submit time (no-reference-input); this + // documents why. + const getJob = vi + .fn() + .mockResolvedValueOnce(jobStatus('jr', 'running')) + .mockResolvedValueOnce(jobStatus('jr', 'success')); + const provider = makeProvider({ + listJobHistory: vi.fn().mockResolvedValue({ + jobs: [handle({ state: 'running', finishedAt: undefined })], + nextCursor: null, + }), + getJob, + getJobHistoryDetail: vi.fn().mockResolvedValue({ + jobId: 'jr', + log: [], + parameters: { + inputTools: { type: 'annotations', uris: ['/f/staged'] }, + }, + }), + getResults: vi.fn().mockResolvedValue(resultsBundle(sampleResults)), + }); + const store = arrange(provider); + const listener = vi.fn(); + store.onJobComplete(listener); + + await store.adoptJobHistory(); + await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS); + + expect(listener.mock.calls[0][0].context?.activeDatasetId).toBeUndefined(); + }); + it('loadJobResults on an adopted terminal job reconstructs its parent', async () => { const provider = makeProvider({ listJobHistory: vi.fn().mockResolvedValue({ diff --git a/src/processing/annotationKinds.ts b/src/processing/annotationKinds.ts new file mode 100644 index 000000000..f0b861553 --- /dev/null +++ b/src/processing/annotationKinds.ts @@ -0,0 +1,16 @@ +// Shared mapping between wire annotation kinds and their client stores. + +import type { AnnotationToolKind } from '@/backend-contract'; +import { useAnnotationToolStore } from '@/src/store/tools'; +import { AnnotationToolType } from '@/src/store/tools/types'; +import type { AnnotationToolStore } from '@/src/store/tools/useAnnotationTool'; + +const ANNOTATION_TOOL_TYPE: Record = { + rulers: AnnotationToolType.Ruler, + rectangles: AnnotationToolType.Rectangle, + polygons: AnnotationToolType.Polygon, +}; + +export const annotationToolStore = ( + kind: AnnotationToolKind +): AnnotationToolStore => useAnnotationToolStore(ANNOTATION_TOOL_TYPE[kind]); diff --git a/src/processing/components/JobsModule.vue b/src/processing/components/JobsModule.vue index 520bfd747..b1c0acadf 100644 --- a/src/processing/components/JobsModule.vue +++ b/src/processing/components/JobsModule.vue @@ -144,6 +144,20 @@ import { mintLabelmapReferenceImage, type SegmentGroupView, } from '@/src/processing/engine/mintLabelmap'; +import { mintInputValue } from '@/src/processing/engine/mintInput'; +import { mintAnnotationsValue } from '@/src/processing/engine/mintAnnotations'; +import { + annotationToolsViewCount, + annotationsFileCount, + encodeAnnotationsFile, + hasTwoPoints, + isEncodablePolygon, + type AnnotationKindView, + type AnnotationToolsView, + type PolygonToolView, + type TwoPointToolView, +} from '@/src/processing/engine/annotationsWire'; +import { annotationToolStore } from '@/src/processing/annotationKinds'; import { bindSourceRefs, type BoundSourceRefType, @@ -155,8 +169,19 @@ import { useSegmentGroupStore } from '@/src/store/segmentGroups'; import { useMessageStore } from '@/src/store/messages'; import { writeSegmentation } from '@/src/io/readWriteImage'; import { getDataSourceName } from '@/src/io/import/dataSource'; -import type { InputValue, VolViewTaskParameter } from '@/backend-contract'; -import { TYPE_TAG_LABELMAP } from '@/backend-contract'; +import type { + AnnotationToolKind, + AnnotationsFile, + InputValue, + VolViewTaskParameter, +} from '@/backend-contract'; +import { + ANNOTATIONS_FILE_EXTENSION, + TYPE_TAG_ANNOTATIONS, + TYPE_TAG_LABELMAP, +} from '@/backend-contract'; +import type { AnnotationTool } from '@/src/types/annotation-tool'; +import { stripExtension } from '@/src/utils/path'; import TaskPicker from './TaskPicker.vue'; import TaskForm from './TaskForm.vue'; @@ -349,25 +374,48 @@ async function onSubmit(values: Record) { // Display formatting reads live active image and segment-group state, so it // must render before the staging await. const display = buildJobDisplay(model, finalValues); + // Same reason, and it also outlives the staging awaits below: the annotations + // file, its name, and its reference image all come from the active image, + // which the user may switch while an upload is in flight. Only encoded when + // something is actually bound to it. + const annotations = + bindings.annotations.parameters.length > 0 + ? captureAnnotationsPayload() + : null; submitting.value = true; - // The store does not surface staging failures, so report them here. - let stagedValues: Record; + // The store does not surface staging failures, so report them here, each + // under its own message. + const stage = async ( + message: string, + run: () => Promise> + ): Promise> => { + try { + return await run(); + } catch (err) { + messageStore.addError(message, { error: ensureError(err) }); + throw err; + } + }; + + // Concurrent: the two stages write disjoint parameter keys, and the + // annotations payload was snapshotted above. + let staged: Record[]; try { - stagedValues = await stageLabelmapInputs( - submitProvider, - model, - finalValues, - bindings - ); - } catch (err) { - messageStore.addError('Failed to stage segment group input', { - error: ensureError(err), - }); + staged = await Promise.all([ + stage('Failed to stage segment group input', () => + stageLabelmapInputs(submitProvider, model, bindings) + ), + stage('Failed to stage annotations input', () => + stageAnnotationInputs(submitProvider, bindings, annotations) + ), + ]); + } catch { submitting.value = false; return; } + const stagedValues = { ...finalValues, ...staged[0], ...staged[1] }; try { await providers.submitJob(providerId, taskId, stagedValues, { @@ -439,18 +487,55 @@ function labelmapReferenceImage(segmentGroupId: string): InputValue | null { ); } +// Tool lists are per image, and so is the staged annotations file: only the +// active image's finished tools are ever an input. +function onActiveImage( + tools: readonly T[] +): T[] { + const id = currentImageID.value; + return id ? tools.filter((tool) => tool.imageID === id) : []; +} + +// The three stores are independent, so each keeps its own label namespace; the +// encoder prunes and re-keys them by name. +const annotationToolsView = computed(() => { + const kindView = ( + kind: AnnotationToolKind, + hasGeometry: (tool: U) => tool is U & T + ): AnnotationKindView => { + const store = annotationToolStore(kind); + return { + tools: onActiveImage(store.finishedTools).filter(hasGeometry), + labels: store.labels, + }; + }; + return { + rulers: kindView('rulers', hasTwoPoints), + rectangles: kindView('rectangles', hasTwoPoints), + polygons: kindView('polygons', isEncodablePolygon), + }; +}); + +// Computed, not a function call: placing a tool churns the stores every drag +// frame, and an unchanged count stops the invalidation there. +const finishedAnnotationCount = computed(() => + annotationToolsViewCount(annotationToolsView.value) +); + function activeSourceBindings(model: TaskFormModel): SourceRefBindings { return bindSourceRefs(model, { activeDataSource: activeDataSource(), backgroundImageId: currentImageID.value ?? undefined, activeSegmentGroupId: paintStore.activeSegmentGroupID, segmentGroups: segmentGroupView(), + hasFinishedAnnotations: finishedAnnotationCount.value > 0, getDataSource: (imageId) => datasetStore.getDataSource(imageId), }); } -// The labelmap value is not set here: a segment group has no server provenance, -// so it earns URIs only at Run via `stageLabelmapInputs`. +// Neither the labelmap nor the annotations value is set here: neither has +// server provenance of its own, so both earn URIs only at Run via the staging +// helpers below. function applyActiveBindings( model: TaskFormModel, base: Record, @@ -497,16 +582,17 @@ function refreshValidation( return validation.issues; } +// Returns only the parameters it staged, so the caller owns the merge. +// // The literal 'seg.nrrd' name is required for segment names and colors to be // embedded in the serialized output. async function stageLabelmapInputs( p: ProcessingProvider, model: TaskFormModel, - values: Record, bindings: SourceRefBindings = activeSourceBindings(model) ): Promise> { const targets = Object.entries(bindings.labelmap.groups); - if (targets.length === 0) return values; + if (targets.length === 0) return {}; const staged = await Promise.all( targets.map(async ([parameterId, segmentGroupId]) => { @@ -538,7 +624,68 @@ async function stageLabelmapInputs( return [parameterId, mintLabelmapValue(uris)] as const; }) ); - return { ...values, ...Object.fromEntries(staged) }; + return Object.fromEntries(staged); +} + +// The extension is what the CLI spec and the backend both match on, so the base +// name is the active image's without its own — compound extensions included, so +// `scan.nii.gz` stages as `scan.annotations.json`. +function annotationsFileName(): string { + const name = activeImageName() ?? 'image'; + return `${stripExtension(name)}${ANNOTATIONS_FILE_EXTENSION}`; +} + +// Everything the annotations file is made of, read off the stores in one +// synchronous pass so staging never mixes two images' state. +type AnnotationsPayload = { + file: AnnotationsFile; + name: string; + referenceImage: InputValue | null; +}; + +function captureAnnotationsPayload(): AnnotationsPayload { + return { + file: encodeAnnotationsFile(annotationToolsView.value), + name: annotationsFileName(), + referenceImage: mintInputValue(activeDataSource()), + }; +} + +// One file per bound parameter, holding every finished tool the active image +// had at Run. The image is its own reference image, so a volume without server +// provenance never gets here — the binder already refused it. +async function stageAnnotationInputs( + p: ProcessingProvider, + bindings: SourceRefBindings, + payload: AnnotationsPayload | null +): Promise> { + const [parameterId] = bindings.annotations.parameters; + if (!parameterId || !payload) return {}; + const { file, name, referenceImage } = payload; + + if (!referenceImage) { + throw new Error('The active image has no server provenance'); + } + // The binder validated a live count; this is the encoded file's own count, so + // a tool deleted between validation and Run cannot stage an empty file. + if (annotationsFileCount(file) === 0) { + throw new Error('The active image has no finished annotations'); + } + + const uris = await p.stageInput({ + file: new Blob([JSON.stringify(file)], { type: 'application/json' }), + descriptor: { + type: TYPE_TAG_ANNOTATIONS, + name, + referenceImage: { + ...referenceImage, + type: 'image', + }, + }, + }); + return { + [parameterId]: mintAnnotationsValue(uris), + }; } // Binding can fall back to the current image's sole group, so reading @@ -547,6 +694,7 @@ type SourceRefContext = { labelmapGroups: Record; types: Record; imageName: string | undefined; + annotationCount: number; }; // Resolved once per display pass: each binding re-runs a full field scan and @@ -557,6 +705,7 @@ function sourceRefContext(model: TaskFormModel): SourceRefContext { labelmapGroups: bindings.labelmap.groups, types: bindings.types, imageName: activeImageName(), + annotationCount: finishedAnnotationCount.value, }; } @@ -568,6 +717,24 @@ function boundLabelmapName( return groupId ? segmentGroupStore.metadataByID[groupId]?.name : undefined; } +// The bound value is a whole set of tools rather than one named resource, so +// the count is the identifying part. +function boundAnnotationsName(refs: SourceRefContext): string { + const noun = refs.annotationCount === 1 ? 'annotation' : 'annotations'; + const count = `${refs.annotationCount} ${noun}`; + return refs.imageName ? `${count} on ${refs.imageName}` : count; +} + +function boundSourceRefName( + refs: SourceRefContext, + parameterId: string +): string | undefined { + const type = refs.types[parameterId]; + if (type === TYPE_TAG_LABELMAP) return boundLabelmapName(refs, parameterId); + if (type === TYPE_TAG_ANNOTATIONS) return boundAnnotationsName(refs); + return refs.imageName; +} + const sourceRefNames = computed(() => { const model = taskModel.value; if (!model) return {}; @@ -575,10 +742,7 @@ const sourceRefNames = computed(() => { const names: Record = {}; model.fields.forEach((field) => { if (field.kind !== 'sourceRef') return; - const name = - refs.types[field.id] === TYPE_TAG_LABELMAP - ? boundLabelmapName(refs, field.id) - : refs.imageName; + const name = boundSourceRefName(refs, field.id); if (name) names[field.id] = name; }); return names; @@ -593,6 +757,9 @@ function formatProcessingValue( if (refs.types[field.id] === TYPE_TAG_LABELMAP) { return boundLabelmapName(refs, field.id) ?? 'bound segment group'; } + if (refs.types[field.id] === TYPE_TAG_ANNOTATIONS) { + return boundAnnotationsName(refs); + } return refs.imageName ?? 'active dataset'; } if (field.kind === 'bounds') { @@ -657,6 +824,9 @@ watchDebounced( crop: id ? cropStore.croppingByImageID[id] : undefined, activeSegmentGroup: paintStore.activeSegmentGroupID, groupCount: id ? (segmentGroupStore.orderByParent[id]?.length ?? 0) : 0, + // Placing the first (or removing the last) tool flips the annotations + // binding, so the form must revalidate. + annotationCount: finishedAnnotationCount.value, }; }, () => { diff --git a/src/processing/components/__tests__/JobsModule.spec.ts b/src/processing/components/__tests__/JobsModule.spec.ts index 236a1d051..c97c97c75 100644 --- a/src/processing/components/__tests__/JobsModule.spec.ts +++ b/src/processing/components/__tests__/JobsModule.spec.ts @@ -25,6 +25,9 @@ import JobsModule from '@/src/processing/components/JobsModule.vue'; 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 { useRulerStore } from '@/src/store/tools/rulers'; +import { useViewStore } from '@/src/store/views'; const cfg = (id: string): ProcessingProviderConfig => ({ id, @@ -327,4 +330,80 @@ describe('JobsModule — race-free provider/task selection', () => { expect(vm.taskError).toBeNull(); expect(vm.taskModel?.id).toBe('x'); }); + + it('rebinds an annotations input after the first ruler is finished', async () => { + const p = makeProvider('P'); + p.listTasks = vi + .fn() + .mockResolvedValue([ + { id: 'RulerToRectangle', title: 'Ruler to Rectangle' }, + ]); + p.getTaskSpec = vi.fn().mockResolvedValue({ + specVersion: 1, + id: 'RulerToRectangle', + title: 'Ruler to Rectangle', + parameters: [ + { + kind: 'sourceRef', + id: 'inputVolume', + accepts: ['image'], + required: true, + }, + { + kind: 'sourceRef', + id: 'inputAnnotations', + title: 'Input Annotations', + accepts: ['annotations'], + required: true, + }, + ], + outputs: [], + }); + + const store = useProcessingJobsStore(); + registerFake(store, p); + useDatasetStore().addDataSources([ + { + dataID: 'image-1', + dataSource: { + type: 'uri', + uri: 'girder://file/image-1', + name: 'image.nrrd', + }, + }, + ]); + useViewStore().setDataForAllViews('image-1'); + + const wrapper = mount(); + await flushPromises(); + expect(wrapper.findComponent(TaskForm).props('issues')).toEqual([ + expect.objectContaining({ + parameter: 'inputAnnotations', + message: expect.stringMatching(/place a ruler/i), + }), + ]); + + useRulerStore().addRuler({ + imageID: 'image-1', + name: 'Ruler', + firstPoint: [0, 0, 0], + secondPoint: [1, 1, 0], + frameOfReference: { + planeNormal: [0, 0, 1], + planeOrigin: [0, 0, 0], + }, + slice: 0, + placing: false, + }); + + await new Promise((resolve) => setTimeout(resolve, 200)); + await flushPromises(); + + const form = wrapper.findComponent(TaskForm); + expect(form.props('issues')).toEqual([]); + expect(form.props('sourceRefStates')).toMatchObject({ + inputVolume: 'bound', + inputAnnotations: 'bound', + }); + }); }); diff --git a/src/processing/components/widgets/FileWidget.vue b/src/processing/components/widgets/FileWidget.vue index 5f60ae918..9f784d823 100644 --- a/src/processing/components/widgets/FileWidget.vue +++ b/src/processing/components/widgets/FileWidget.vue @@ -5,15 +5,11 @@ @@ -22,10 +18,11 @@ diff --git a/src/processing/engine/__tests__/annotationsWire.spec.ts b/src/processing/engine/__tests__/annotationsWire.spec.ts new file mode 100644 index 000000000..2448eede9 --- /dev/null +++ b/src/processing/engine/__tests__/annotationsWire.spec.ts @@ -0,0 +1,530 @@ +import { describe, expect, it } from 'vitest'; + +import { annotationsFileSchema } from '@/backend-contract'; +import { loadFixture } from '@/backend-contract/processing/__tests__/loadFixtures'; +import { + annotationToolsViewCount, + annotationsFileCount, + decodeAnnotationsFile, + encodeAnnotationsFile, + type AnnotationToolsView, + type PolygonToolView, + type TwoPointToolView, +} from '../annotationsWire'; + +const emptyAnnotationToolsView = (): AnnotationToolsView => ({ + rulers: { tools: [], labels: {} }, + rectangles: { tools: [], labels: {} }, + polygons: { tools: [], labels: {} }, +}); + +// The contract's golden interchange file, read off disk so the client decoder +// and the backend's JSON Schema validate the exact same bytes. +// Typed loosely on purpose: the negative cases mutate it into the very shapes +// the schema must reject. +type GoldenTool = Record & { + frameOfReference?: { planeNormal: number[] }; +}; + +type GoldenFile = { + tools: Record; + [key: string]: unknown; +}; + +const goldenFixture = () => + loadFixture('wire/annotations-file.json') as GoldenFile; + +const axialFrame = { + planeNormal: [0, 0, 1] as [number, number, number], + planeOrigin: [0, 0, -12.5] as [number, number, number], +}; + +// A store tool as it really is: geometry and core plus every session-only field +// that must not reach the wire. +const sessionRuler = ( + overrides: Partial = {} +): TwoPointToolView => + ({ + id: 'tool-1', + imageID: 'image-1', + firstPoint: [-30.5, 12.25, -12.5], + secondPoint: [18.75, 44, -12.5], + frameOfReference: axialFrame, + slice: 42, + label: 'label-id-1', + labelName: 'lesion', + name: 'Ruler', + color: '#ff0000', + strokeWidth: 2, + fillColor: '#00ff0033', + hidden: false, + placing: false, + source: { providerId: 'p', jobId: 'j', outputId: 'o' }, + ...overrides, + }) as TwoPointToolView; + +const sessionPolygon = ( + overrides: Partial = {} +): PolygonToolView => + ({ + id: 'tool-3', + imageID: 'image-1', + points: [ + [-20, 0, -12.5], + [10, 0, -12.5], + [10, 30, -12.5], + ], + frameOfReference: axialFrame, + slice: 42, + labelName: 'roi', + name: 'Polygon', + color: '#0000ff', + placing: false, + ...overrides, + }) as PolygonToolView; + +const viewOf = (overrides: Partial): AnnotationToolsView => + ({ ...emptyAnnotationToolsView(), ...overrides }) as AnnotationToolsView; + +// --------------------------------------------------------------------------- +// Encode: the session/wire boundary +// --------------------------------------------------------------------------- + +describe('encodeAnnotationsFile', () => { + it('stamps the fail-closed envelope', () => { + const file = encodeAnnotationsFile(emptyAnnotationToolsView()); + expect(file.schemaVersion).toBe(1); + expect(file.space).toBe('LPS'); + }); + + it('drops every session-only field from a tool', () => { + const file = encodeAnnotationsFile( + viewOf({ + rulers: { + tools: [sessionRuler()], + labels: { 'label-id-1': { labelName: 'lesion', color: '#ff0000' } }, + }, + }) + ); + + const [ruler] = file.tools.rulers!; + expect(Object.keys(ruler).sort()).toEqual([ + 'firstPoint', + 'frameOfReference', + 'labelName', + 'name', + 'secondPoint', + 'slice', + ]); + expect(ruler).not.toHaveProperty('id'); + expect(ruler).not.toHaveProperty('imageID'); + expect(ruler).not.toHaveProperty('color'); + expect(ruler).not.toHaveProperty('strokeWidth'); + expect(ruler).not.toHaveProperty('fillColor'); + expect(ruler).not.toHaveProperty('hidden'); + expect(ruler).not.toHaveProperty('placing'); + expect(ruler).not.toHaveProperty('label'); + expect(ruler).not.toHaveProperty('source'); + }); + + it('carries geometry, the frame, and the advisory core', () => { + const file = encodeAnnotationsFile( + viewOf({ + rulers: { + tools: [ + sessionRuler({ + frame: 3, + metadata: { measuredBy: 'reader-1' }, + }), + ], + labels: { 'label-id-1': { labelName: 'lesion' } }, + }, + }) + ); + + expect(file.tools.rulers![0]).toMatchObject({ + firstPoint: [-30.5, 12.25, -12.5], + secondPoint: [18.75, 44, -12.5], + frameOfReference: { + planeNormal: [0, 0, 1], + planeOrigin: [0, 0, -12.5], + }, + slice: 42, + frame: 3, + labelName: 'lesion', + name: 'Ruler', + metadata: { measuredBy: 'reader-1' }, + }); + }); + + it('omits the unset-slice sentinel rather than echoing a lie', () => { + const file = encodeAnnotationsFile( + viewOf({ + rulers: { + tools: [sessionRuler({ slice: -1, labelName: '' })], + labels: {}, + }, + }) + ); + expect(file.tools.rulers![0]).not.toHaveProperty('slice'); + expect(file.tools.rulers![0]).not.toHaveProperty('labelName'); + }); + + it.each([ + [undefined, undefined], + [0, 0], + [3, 3], + [-1, undefined], + [1.5, undefined], + [Number.NaN, undefined], + [Number.POSITIVE_INFINITY, undefined], + [Number.NEGATIVE_INFINITY, undefined], + ])( + 'projects session frame %s to a contract-valid wire value', + (frame, expected) => { + const file = encodeAnnotationsFile( + viewOf({ + rulers: { + tools: [sessionRuler({ frame })], + labels: { 'label-id-1': { labelName: 'lesion' } }, + }, + }) + ); + + expect(file.tools.rulers![0].frame).toBe(expected); + expect(annotationsFileSchema.safeParse(file).success).toBe(true); + } + ); + + it('keeps the same label name independent per tool kind', () => { + const file = encodeAnnotationsFile( + viewOf({ + rulers: { + tools: [sessionRuler()], + labels: { + 'ruler-label': { + labelName: 'lesion', + color: '#ff0000', + strokeWidth: 2, + }, + }, + }, + rectangles: { + tools: [sessionRuler({ id: 'tool-2' } as Partial)], + labels: { + 'rect-label': { + labelName: 'lesion', + color: '#00ff00', + strokeWidth: 1, + fillColor: '#00ff0033', + }, + }, + }, + }) + ); + + expect(file.labels!.rulers).toEqual({ + lesion: { color: '#ff0000', strokeWidth: 2 }, + }); + expect(file.labels!.rectangles).toEqual({ + lesion: { color: '#00ff00', strokeWidth: 1, fillColor: '#00ff0033' }, + }); + }); + + it('keeps a rectangle fill style in the label namespace, never on the tool', () => { + const file = encodeAnnotationsFile( + viewOf({ + rectangles: { + tools: [sessionRuler({ name: 'Rectangle' })], + labels: { + 'rect-label': { labelName: 'lesion', fillColor: '#00ff0033' }, + }, + }, + }) + ); + expect(file.tools.rectangles![0]).not.toHaveProperty('fillColor'); + expect(file.labels!.rectangles!.lesion.fillColor).toBe('#00ff0033'); + }); + + it('prunes label namespaces to the names the tools reference', () => { + const file = encodeAnnotationsFile( + viewOf({ + rulers: { + tools: [sessionRuler()], + labels: { + 'ruler-label': { labelName: 'lesion', color: '#ff0000' }, + unused: { labelName: 'tumor', color: '#123456' }, + }, + }, + }) + ); + expect(Object.keys(file.labels!.rulers!)).toEqual(['lesion']); + }); + + it('declares a referenced name with no store label rather than dangling', () => { + const file = encodeAnnotationsFile( + viewOf({ + rulers: { tools: [sessionRuler({ labelName: 'orphan' })], labels: {} }, + }) + ); + expect(file.labels!.rulers).toEqual({ orphan: {} }); + // Fails closed downstream if it were dangling. + expect(() => decodeAnnotationsFile(file)).not.toThrow(); + }); + + it('skips a polygon with fewer than three points', () => { + const file = encodeAnnotationsFile( + viewOf({ + polygons: { + tools: [ + sessionPolygon({ + points: [ + [0, 0, 0], + [1, 1, 0], + ], + }), + sessionPolygon(), + ], + labels: { 'poly-label': { labelName: 'roi', color: '#0000ff' } }, + }, + }) + ); + expect(file.tools.polygons).toHaveLength(1); + expect(file.tools.polygons![0].points).toHaveLength(3); + }); + + // Session geometry the contract forbids is caught here rather than as a 400. + it('refuses a non-finite coordinate', () => { + expect(() => + encodeAnnotationsFile( + viewOf({ + rulers: { + tools: [sessionRuler({ firstPoint: [Number.NaN, 0, 0] })], + labels: { 'label-id-1': { labelName: 'lesion' } }, + }, + }) + ) + ).toThrow(); + }); + + it('refuses a degenerate plane normal', () => { + expect(() => + encodeAnnotationsFile( + viewOf({ + polygons: { + tools: [ + sessionPolygon({ + frameOfReference: { + planeNormal: [0, 0, 0], + planeOrigin: [0, 0, 0], + }, + }), + ], + labels: { 'poly-label': { labelName: 'roi' } }, + }, + }) + ) + ).toThrow(/nonzero vector/); + }); + + it('emits an empty but valid file when nothing is placed', () => { + const file = encodeAnnotationsFile(emptyAnnotationToolsView()); + expect(file.tools).toEqual({}); + expect(file).not.toHaveProperty('labels'); + expect(() => decodeAnnotationsFile(file)).not.toThrow(); + }); +}); + +describe('annotationToolsViewCount', () => { + it('counts finished tools across all three kinds', () => { + expect(annotationToolsViewCount(emptyAnnotationToolsView())).toBe(0); + expect( + annotationToolsViewCount( + viewOf({ + rulers: { tools: [sessionRuler(), sessionRuler()], labels: {} }, + polygons: { tools: [sessionPolygon()], labels: {} }, + }) + ) + ).toBe(3); + }); +}); + +describe('annotationsFileCount', () => { + it('counts what the file carries, not what the view held', () => { + const view = viewOf({ + rulers: { tools: [sessionRuler()], labels: {} }, + polygons: { + tools: [ + sessionPolygon({ + points: [ + [0, 0, 0], + [1, 1, 0], + ], + }), + ], + labels: {}, + }, + }); + // The half-placed polygon is dropped on encode, so the two counts differ. + expect(annotationToolsViewCount(view)).toBe(2); + expect(annotationsFileCount(encodeAnnotationsFile(view))).toBe(1); + }); + + it('is zero for a file with nothing placed', () => { + expect( + annotationsFileCount(encodeAnnotationsFile(emptyAnnotationToolsView())) + ).toBe(0); + }); +}); + +// --------------------------------------------------------------------------- +// Decode: fail closed, then project +// --------------------------------------------------------------------------- + +describe('decodeAnnotationsFile', () => { + it('decodes the contract golden fixture', () => { + const decoded = decodeAnnotationsFile(goldenFixture()); + + expect(decoded.tools.rulers).toHaveLength(1); + expect(decoded.tools.rectangles).toHaveLength(1); + expect(decoded.tools.polygons).toHaveLength(1); + expect(decoded.tools.rulers[0].metadata).toEqual({ + measuredBy: 'reader-1', + }); + // The same name, two independent styles. + expect(decoded.labels.rulers.lesion).toEqual({ + color: '#ff0000', + strokeWidth: 2, + }); + expect(decoded.labels.rectangles.lesion).toEqual({ + color: '#00ff00', + strokeWidth: 1, + fillColor: '#00ff0033', + }); + expect(decoded.labels.polygons.roi).toEqual({ color: '#0000ff' }); + }); + + it('normalizes absent kinds to empty arrays and namespaces', () => { + const decoded = decodeAnnotationsFile({ + schemaVersion: 1, + space: 'LPS', + tools: {}, + }); + expect(decoded.tools).toEqual({ rulers: [], rectangles: [], polygons: [] }); + expect(decoded.labels).toEqual({ + rulers: {}, + rectangles: {}, + polygons: {}, + }); + }); + + it.each([ + [ + [0, 0, 2], + [0, 0, 1], + ], + [ + [0, 0, -3], + [0, 0, -1], + ], + [ + [0, 0, 0.99999], + [0, 0, 1], + ], + ])('normalizes a nonzero plane normal %j', (planeNormal, expected) => { + const file = goldenFixture(); + file.tools.rulers[0].frameOfReference!.planeNormal = planeNormal; + + const decoded = decodeAnnotationsFile(file); + + expect(decoded.tools.rulers[0].frameOfReference.planeNormal).toEqual( + expected + ); + }); + + it('rejects a zero plane normal', () => { + const file = goldenFixture(); + file.tools.rulers[0].frameOfReference!.planeNormal = [0, 0, 0]; + expect(() => decodeAnnotationsFile(file)).toThrow(/nonzero vector/); + }); + + it('rejects a foreign schemaVersion or space', () => { + const golden = goldenFixture(); + expect(() => + decodeAnnotationsFile({ ...golden, schemaVersion: 2 }) + ).toThrow(/annotations file/i); + expect(() => decodeAnnotationsFile({ ...golden, space: 'RAS' })).toThrow( + /annotations file/i + ); + }); + + it('rejects a dangling label reference', () => { + const golden = goldenFixture(); + golden.tools.rulers[0].labelName = 'not-declared'; + expect(() => decodeAnnotationsFile(golden)).toThrow(/not-declared/); + }); + + it('rejects a dangling reference across namespaces', () => { + const golden = goldenFixture(); + // 'roi' is declared for polygons only. + golden.tools.rulers[0].labelName = 'roi'; + expect(() => decodeAnnotationsFile(golden)).toThrow(/labels\.rulers/); + }); + + it('rejects a session-only field smuggled onto a tool', () => { + const golden = goldenFixture(); + golden.tools.rulers[0].imageID = 'image-1'; + expect(() => decodeAnnotationsFile(golden)).toThrow(/annotations file/i); + }); + + it('rejects a polygon with fewer than three points', () => { + const golden = goldenFixture(); + golden.tools.polygons[0].points = [ + [0, 0, 0], + [1, 1, 0], + ]; + expect(() => decodeAnnotationsFile(golden)).toThrow(/annotations file/i); + }); + + it('rejects a tool with no frame of reference', () => { + const golden = goldenFixture(); + delete golden.tools.rulers[0].frameOfReference; + expect(() => decodeAnnotationsFile(golden)).toThrow(/annotations file/i); + }); + + it('drops an unrecognized envelope field the schema lets through', () => { + const golden = goldenFixture(); + golden.producer = 'some-cli'; + const decoded = decodeAnnotationsFile(golden); + expect(decoded).not.toHaveProperty('producer'); + }); + + it('round-trips an encoded file', () => { + const view = viewOf({ + rulers: { + tools: [sessionRuler()], + labels: { 'ruler-label': { labelName: 'lesion', color: '#ff0000' } }, + }, + rectangles: { + tools: [sessionRuler({ name: 'Rectangle' })], + labels: { + 'rect-label': { labelName: 'lesion', fillColor: '#00ff0033' }, + }, + }, + polygons: { + tools: [sessionPolygon()], + labels: { 'poly-label': { labelName: 'roi', color: '#0000ff' } }, + }, + }); + + const encoded = encodeAnnotationsFile(view); + const decoded = decodeAnnotationsFile(JSON.parse(JSON.stringify(encoded))); + + expect(decoded.tools.rulers).toEqual(encoded.tools.rulers); + expect(decoded.tools.rectangles).toEqual(encoded.tools.rectangles); + expect(decoded.tools.polygons).toEqual(encoded.tools.polygons); + expect(decoded.labels.rulers).toEqual(encoded.labels!.rulers); + expect(decoded.labels.rectangles).toEqual(encoded.labels!.rectangles); + expect(decoded.labels.polygons).toEqual(encoded.labels!.polygons); + }); +}); diff --git a/src/processing/engine/__tests__/mintAnnotations.spec.ts b/src/processing/engine/__tests__/mintAnnotations.spec.ts new file mode 100644 index 000000000..41fee2321 --- /dev/null +++ b/src/processing/engine/__tests__/mintAnnotations.spec.ts @@ -0,0 +1,168 @@ +import { describe, expect, it } from 'vitest'; + +import { + annotationsInputFields, + bindAnnotationsInputs, + mintAnnotationsValue, +} from '../mintAnnotations'; +import type { FormField, TaskFormModel } from '../formModel'; + +const modelOf = (fields: TaskFormModel['fields']): TaskFormModel => ({ + id: 'task', + title: 'Task', + fields, + hidden: [], +}); + +const annotationsField = ( + overrides: Partial> = {} +): FormField => ({ + kind: 'sourceRef', + id: 'inputAnnotations', + accepts: ['annotations'], + required: true, + ...overrides, +}); + +const annotationsModel = ( + overrides: Partial> = {} +): TaskFormModel => modelOf([annotationsField(overrides)]); + +describe('annotationsInputFields', () => { + it('selects sourceRef params that accept annotations', () => { + const model = modelOf([ + { kind: 'sourceRef', id: 'bg', accepts: ['image'], required: true }, + { kind: 'sourceRef', id: 'seg', accepts: ['labelmap'], required: true }, + annotationsField({ id: 'ann' }), + { kind: 'int', id: 'radius', default: 1 }, + ]); + expect(annotationsInputFields(model).map((f) => f.id)).toEqual(['ann']); + }); +}); + +describe('bindAnnotationsInputs — the binding states', () => { + it('is a no-op when the task has no annotations input', () => { + expect( + bindAnnotationsInputs( + modelOf([{ kind: 'int', id: 'n', default: 1 }]), + true, + true, + true + ) + ).toEqual({ parameters: [], states: {}, issues: [] }); + }); + + it('binds the sole annotations param when tools and provenance exist', () => { + const result = bindAnnotationsInputs(annotationsModel(), true, true, true); + expect(result.states.inputAnnotations).toBe('bound'); + expect(result.parameters).toEqual(['inputAnnotations']); + expect(result.issues).toEqual([]); + }); + + it('fails closed (no-annotations) so an empty file is never submitted', () => { + const result = bindAnnotationsInputs(annotationsModel(), false, true, true); + expect(result.states.inputAnnotations).toBe('no-annotations'); + expect(result.parameters).toEqual([]); + expect(result.issues).toHaveLength(1); + expect(result.issues[0].parameter).toBe('inputAnnotations'); + expect(result.issues[0].message).toMatch( + /place a ruler, rectangle, or polygon/i + ); + }); + + it('does not block an OPTIONAL annotations input with nothing placed', () => { + const result = bindAnnotationsInputs( + annotationsModel({ required: false }), + false, + true, + true + ); + expect(result.states.inputAnnotations).toBe('no-annotations'); + expect(result.issues).toEqual([]); + }); + + it('fails closed (no-provenance) when the active image is local, even if optional', () => { + const result = bindAnnotationsInputs( + annotationsModel({ required: false }), + true, + false, + true + ); + expect(result.states.inputAnnotations).toBe('no-provenance'); + expect(result.parameters).toEqual([]); + expect(result.issues).toHaveLength(1); + expect(result.issues[0].message).toMatch(/not loaded from the server/i); + }); + + it('reports the missing annotations before the missing provenance', () => { + const result = bindAnnotationsInputs( + annotationsModel(), + false, + false, + true + ); + expect(result.states.inputAnnotations).toBe('no-annotations'); + }); + + it('fails closed (no-reference-input) when the task declares no image input', () => { + // Adopted-job reconstruction re-identifies the parent from the persisted + // image input; without one the results are unloadable after a reload. + const result = bindAnnotationsInputs(annotationsModel(), true, true, false); + expect(result.states.inputAnnotations).toBe('no-reference-input'); + expect(result.parameters).toEqual([]); + expect(result.issues).toHaveLength(1); + expect(result.issues[0].message).toMatch( + /does not declare the reference image/i + ); + }); + + it('does not block an OPTIONAL annotations input on a task without an image input', () => { + // The defect still refuses the binding, but an optional field means the + // task can run without annotations rather than never at all. + const result = bindAnnotationsInputs( + annotationsModel({ required: false }), + true, + true, + false + ); + expect(result.states.inputAnnotations).toBe('no-reference-input'); + expect(result.parameters).toEqual([]); + expect(result.issues).toEqual([]); + }); + + it('reports the task-shape defect before the scene states', () => { + // A structural defect cannot be fixed by placing tools, so its message + // must not be masked by no-annotations or no-provenance. + const result = bindAnnotationsInputs( + annotationsModel(), + false, + false, + false + ); + expect(result.states.inputAnnotations).toBe('no-reference-input'); + }); + + it('fails closed (ambiguous) when more than one annotations param is present', () => { + const result = bindAnnotationsInputs( + modelOf([ + annotationsField({ id: 'annA' }), + annotationsField({ id: 'annB' }), + ]), + true, + true, + true + ); + expect(result.states.annA).toBe('ambiguous'); + expect(result.states.annB).toBe('ambiguous'); + expect(result.parameters).toEqual([]); + expect(result.issues).toHaveLength(1); + expect(result.issues[0].message).toMatch(/more than one annotation input/i); + }); +}); + +describe('mintAnnotationsValue', () => { + it('mints { type: "annotations", uris } from the staging response (no format)', () => { + const uris = ['/api/v1/file/deadbeef/proxiable/scan.annotations.json']; + expect(mintAnnotationsValue(uris)).toEqual({ type: 'annotations', uris }); + }); +}); diff --git a/src/processing/engine/__tests__/sourceRefs.spec.ts b/src/processing/engine/__tests__/sourceRefs.spec.ts index 999b77111..ee4d604f8 100644 --- a/src/processing/engine/__tests__/sourceRefs.spec.ts +++ b/src/processing/engine/__tests__/sourceRefs.spec.ts @@ -23,10 +23,20 @@ const context = ( backgroundImageId: 'image-1', activeSegmentGroupId: null, segmentGroups: { orderByParent: {}, metadataByID: {} }, + hasFinishedAnnotations: false, getDataSource: () => remoteImage, ...overrides, }); +// The single-group arrangement the labelmap resolver binds without a picker. +const oneSegmentGroup = { + activeSegmentGroupId: 'group-1', + segmentGroups: { + orderByParent: { 'image-1': ['group-1'] }, + metadataByID: { 'group-1': { parentImage: 'image-1' } }, + }, +}; + describe('bindSourceRefs', () => { it('uses an image alternative when no labelmap is available', () => { const bindings = bindSourceRefs( @@ -177,3 +187,221 @@ describe('bindSourceRefs', () => { expect(dataSourceReads).toBe(1); }); }); + +describe('bindSourceRefs — annotations', () => { + // The RulerToRectangle shape: the reference image is declared alongside the + // annotations, which is what makes results reloadable in a later session. + const annotationsModel = () => + model([ + { kind: 'sourceRef', id: 'image', accepts: ['image'], required: true }, + { + kind: 'sourceRef', + id: 'annotations', + accepts: ['annotations'], + required: true, + }, + ]); + + const annotationsOnlyModel = () => + model([ + { + kind: 'sourceRef', + id: 'annotations', + accepts: ['annotations'], + required: true, + }, + ]); + + it('binds a dedicated annotations input when tools exist on a remote image', () => { + const bindings = bindSourceRefs( + annotationsModel(), + context({ hasFinishedAnnotations: true }) + ); + + expect(bindings.types.annotations).toBe('annotations'); + expect(bindings.annotations.parameters).toEqual(['annotations']); + expect(bindings.states.annotations).toBe('bound'); + expect(bindings.issues).toEqual([]); + }); + + it('fails closed with nothing placed', () => { + const bindings = bindSourceRefs(annotationsModel(), context()); + + expect(bindings.annotations.parameters).toEqual([]); + expect(bindings.states.annotations).toBe('no-annotations'); + expect(bindings.issues).toHaveLength(1); + expect(bindings.issues[0].message).toMatch(/place a ruler/i); + }); + + it('fails closed when the annotated image has no server provenance', () => { + const bindings = bindSourceRefs( + annotationsModel(), + context({ + hasFinishedAnnotations: true, + activeDataSource: { + type: 'file', + file: new File([], 'local.nrrd'), + fileType: '', + }, + }) + ); + + expect(bindings.annotations.parameters).toEqual([]); + expect(bindings.states.annotations).toBe('no-provenance'); + // Both the image input and the annotations input report the missing + // provenance. + expect(bindings.issues).toHaveLength(2); + }); + + it('fails closed when the task declares no reference image input', () => { + // Adopted-job reconstruction re-identifies the parent from the persisted + // image input, so an annotations-only task would produce results no later + // session could load. Blocks even with tools placed and provenance intact. + const bindings = bindSourceRefs( + annotationsOnlyModel(), + context({ hasFinishedAnnotations: true }) + ); + + expect(bindings.annotations.parameters).toEqual([]); + expect(bindings.states.annotations).toBe('no-reference-input'); + expect(bindings.issues).toHaveLength(1); + expect(bindings.issues[0].message).toMatch( + /does not declare the reference image/i + ); + }); + + it('a staged-type sibling does not count as the reference image', () => { + // Labelmap inputs are excluded from parent reconstruction just like + // annotations, so a labelmap sibling leaves the task unloadable too. + const bindings = bindSourceRefs( + model([ + { kind: 'sourceRef', id: 'seg', accepts: ['labelmap'], required: true }, + { + kind: 'sourceRef', + id: 'annotations', + accepts: ['annotations'], + required: true, + }, + ]), + context({ hasFinishedAnnotations: true, ...oneSegmentGroup }) + ); + + expect(bindings.annotations.parameters).toEqual([]); + expect(bindings.states.annotations).toBe('no-reference-input'); + }); + + it('binds an image and an annotations input from the same active image', () => { + const bindings = bindSourceRefs( + model([ + { kind: 'sourceRef', id: 'image', accepts: ['image'], required: true }, + { + kind: 'sourceRef', + id: 'annotations', + accepts: ['annotations'], + required: true, + }, + ]), + context({ hasFinishedAnnotations: true }) + ); + + expect(bindings.types).toEqual({ + image: 'image', + annotations: 'annotations', + }); + expect(bindings.image.values.image).toMatchObject({ type: 'image' }); + expect(bindings.annotations.parameters).toEqual(['annotations']); + expect(bindings.issues).toEqual([]); + }); + + it('prefers an available annotations alternative over a dedicated image', () => { + const bindings = bindSourceRefs( + model([ + { kind: 'sourceRef', id: 'image', accepts: ['image'], required: true }, + { + kind: 'sourceRef', + id: 'either', + accepts: ['image', 'annotations'], + required: true, + }, + ]), + context({ hasFinishedAnnotations: true }) + ); + + expect(bindings.types).toEqual({ + image: 'image', + either: 'annotations', + }); + expect(bindings.annotations.parameters).toEqual(['either']); + expect(bindings.issues).toEqual([]); + }); + + it('falls back to the image alternative when nothing is placed', () => { + const bindings = bindSourceRefs( + model([ + { + kind: 'sourceRef', + id: 'either', + accepts: ['annotations', 'image'], + required: true, + }, + ]), + context() + ); + + expect(bindings.types.either).toBe('image'); + expect(bindings.annotations.parameters).toEqual([]); + expect(bindings.issues).toEqual([]); + }); + + it('binds all three kinds side by side', () => { + const bindings = bindSourceRefs( + model([ + { kind: 'sourceRef', id: 'image', accepts: ['image'], required: true }, + { kind: 'sourceRef', id: 'seg', accepts: ['labelmap'], required: true }, + { + kind: 'sourceRef', + id: 'annotations', + accepts: ['annotations'], + required: true, + }, + ]), + context({ hasFinishedAnnotations: true, ...oneSegmentGroup }) + ); + + expect(bindings.types).toEqual({ + image: 'image', + seg: 'labelmap', + annotations: 'annotations', + }); + expect(bindings.labelmap.groups.seg).toBe('group-1'); + expect(bindings.annotations.parameters).toEqual(['annotations']); + expect(bindings.issues).toEqual([]); + }); + + it('mints the active image once for an image + annotations task', () => { + let sourceReads = 0; + const source = { + type: 'collection', + get sources() { + sourceReads += 1; + return [remoteImage]; + }, + } as DataSource; + + bindSourceRefs( + model([ + { kind: 'sourceRef', id: 'image', accepts: ['image'], required: true }, + { + kind: 'sourceRef', + id: 'annotations', + accepts: ['annotations'], + required: true, + }, + ]), + context({ activeDataSource: source, hasFinishedAnnotations: true }) + ); + + // One mint reads the collection once for provenance and once for format. + expect(sourceReads).toBe(2); + }); +}); diff --git a/src/processing/engine/annotationsWire.ts b/src/processing/engine/annotationsWire.ts new file mode 100644 index 000000000..ee83edb3f --- /dev/null +++ b/src/processing/engine/annotationsWire.ts @@ -0,0 +1,298 @@ +// Pure projections between annotation stores and `*.annotations.json`. +// Encoding allowlists explicitly, keeping session-only state out of task files; +// decoding leans on the contract's strict schemas, which reject an unrecognized +// producer field rather than let it reach a store. + +import type { + AnnotationLabel, + AnnotationToolKind, + AnnotationsFile, + WirePolygon, + WireRuler, +} from '@/backend-contract'; +import { + ANNOTATIONS_FILE_SCHEMA_VERSION, + ANNOTATIONS_SPACE, + ANNOTATION_TOOL_KINDS, + annotationsFileSchema, +} from '@/backend-contract'; +import { cleanUndefined } from '@/src/utils'; + +// --------------------------------------------------------------------------- +// The store-side view +// --------------------------------------------------------------------------- + +type WireVector3 = [number, number, number]; + +// Points arrive as vtk.js `Vector3`s, which are structurally these tuples; the +// view accepts any 3-number-indexable so callers need no casts. +type PointLike = ArrayLike; + +// A store label as `useLabels` holds it: keyed by label id, carrying the name +// and the style props. `fillColor` is rectangles-only. +export type AnnotationLabelView = { + labelName?: string; + color?: string; + strokeWidth?: number; + fillColor?: string; +}; + +type AnnotationToolCoreView = { + frameOfReference: { planeNormal: PointLike; planeOrigin: PointLike }; + slice?: number; + frame?: number; + labelName?: string; + name?: string; + metadata?: Record; +}; + +export type TwoPointToolView = AnnotationToolCoreView & { + firstPoint: PointLike; + secondPoint: PointLike; +}; + +export type PolygonToolView = AnnotationToolCoreView & { + points: ReadonlyArray; +}; + +export type AnnotationKindView = { + // Finished tools on ONE image; the caller owns that filter. + tools: ReadonlyArray; + // The whole store label map (keyed by label id); encode prunes it. + labels: Record; +}; + +export type AnnotationToolsView = { + rulers: AnnotationKindView; + rectangles: AnnotationKindView; + polygons: AnnotationKindView; +}; + +// Recover geometry omitted by the uniform annotation-store type. +export const hasTwoPoints = ( + tool: T +): tool is T & TwoPointToolView => + 'firstPoint' in tool && 'secondPoint' in tool; + +// A polygon needs three points to bound an area, so a half-placed one is not +// geometry the wire can carry. Excluded here, where the view is built, so the +// count the UI shows and the file the encoder writes agree. +export const isEncodablePolygon = ( + tool: T +): tool is T & PolygonToolView => + 'points' in tool && Array.isArray(tool.points) && tool.points.length >= 3; + +export const annotationToolsViewCount = (view: AnnotationToolsView): number => + ANNOTATION_TOOL_KINDS.reduce( + (total, kind) => total + view[kind].tools.length, + 0 + ); + +// --------------------------------------------------------------------------- +// Encode +// --------------------------------------------------------------------------- + +const toVector3 = (point: PointLike): WireVector3 => [ + point[0], + point[1], + point[2], +]; + +// A plane normal represents a direction, so its magnitude has no wire meaning. +// Normalize at the boundary before axis matching or store insertion. The +// contract's semantic pass rejects zero; keep the guard here so this projection +// remains safe if it is ever called with a value that bypassed that pass. +const normalizePlaneNormal = (normal: PointLike): WireVector3 => { + const magnitude = Math.hypot(normal[0], normal[1], normal[2]); + if (magnitude === 0) { + throw new Error('planeNormal must be a nonzero vector'); + } + return [normal[0] / magnitude, normal[1] / magnitude, normal[2] / magnitude]; +}; + +// `slice` and `frame` are advisory echoes only — a consumer re-derives both +// from `frameOfReference`. A negative slice is the stores' "unset" sentinel and +// would be a lie on the wire. +const encodeCore = (tool: AnnotationToolCoreView) => ({ + frameOfReference: { + planeNormal: toVector3(tool.frameOfReference.planeNormal), + planeOrigin: toVector3(tool.frameOfReference.planeOrigin), + }, + ...cleanUndefined({ + slice: + typeof tool.slice === 'number' && + Number.isFinite(tool.slice) && + tool.slice >= 0 + ? tool.slice + : undefined, + frame: + typeof tool.frame === 'number' && + Number.isInteger(tool.frame) && + tool.frame >= 0 + ? tool.frame + : undefined, + labelName: tool.labelName ? tool.labelName : undefined, + name: tool.name ? tool.name : undefined, + metadata: + tool.metadata && Object.keys(tool.metadata).length > 0 + ? { ...tool.metadata } + : undefined, + }), +}); + +const encodeTwoPointTool = (tool: TwoPointToolView) => ({ + firstPoint: toVector3(tool.firstPoint), + secondPoint: toVector3(tool.secondPoint), + ...encodeCore(tool), +}); + +// A polygon needs three points to bound an area; a half-placed one is dropped +// rather than sent as an invalid file the backend would reject wholesale. +const encodePolygonTool = (tool: PolygonToolView) => ({ + points: tool.points.map(toVector3), + ...encodeCore(tool), +}); + +const encodeLabelStyle = (label: AnnotationLabelView): AnnotationLabel => + cleanUndefined({ + color: label.color, + strokeWidth: label.strokeWidth, + fillColor: label.fillColor, + }); + +// Namespaces are built from the names the encoded tools actually reference, so +// label-reference integrity holds by construction: a referenced name with no +// store entry still gets a (styleless) declaration rather than dangling. +const encodeLabelNamespace = ( + labels: Record, + referenced: Set +): Record => { + const byName = new Map(); + Object.values(labels).forEach((label) => { + if (label?.labelName) byName.set(label.labelName, label); + }); + return Object.fromEntries( + [...referenced].map((labelName) => { + const label = byName.get(labelName); + return [labelName, label ? encodeLabelStyle(label) : {}]; + }) + ); +}; + +export const encodeAnnotationsFile = ( + view: AnnotationToolsView +): AnnotationsFile => { + const rulers = view.rulers.tools.map(encodeTwoPointTool); + const rectangles = view.rectangles.tools.map(encodeTwoPointTool); + const polygons = view.polygons.tools + .filter((tool) => tool.points.length >= 3) + .map(encodePolygonTool); + + const encoded = { rulers, rectangles, polygons }; + const labels = cleanUndefined( + Object.fromEntries( + ANNOTATION_TOOL_KINDS.map((kind) => { + const referenced = new Set( + encoded[kind].flatMap((tool) => + tool.labelName ? [tool.labelName] : [] + ) + ); + const namespace = encodeLabelNamespace(view[kind].labels, referenced); + return [ + kind, + Object.keys(namespace).length > 0 ? namespace : undefined, + ]; + }) + ) as Record | undefined> + ); + + const file = { + schemaVersion: ANNOTATIONS_FILE_SCHEMA_VERSION, + space: ANNOTATIONS_SPACE, + ...(Object.keys(labels).length > 0 ? { labels } : {}), + tools: cleanUndefined({ + rulers: rulers.length > 0 ? rulers : undefined, + rectangles: rectangles.length > 0 ? rectangles : undefined, + polygons: polygons.length > 0 ? polygons : undefined, + }), + }; + + // A session can hold geometry the contract forbids — a non-finite coordinate, + // a degenerate plane normal. Failing here names the offending field, where a + // submitted file would come back as an opaque 400. + return annotationsFileSchema.parse(file); +}; + +// What the file actually carries. Staging checks this rather than the view +// count, so an empty file can never be submitted. +export const annotationsFileCount = (file: AnnotationsFile): number => + ANNOTATION_TOOL_KINDS.reduce( + (total, kind) => total + (file.tools[kind]?.length ?? 0), + 0 + ); + +// --------------------------------------------------------------------------- +// Decode +// --------------------------------------------------------------------------- + +// Normalized: every kind is present as an array, every namespace as a map, so +// consumers never branch on optionality. +export type DecodedAnnotationsFile = { + schemaVersion: typeof ANNOTATIONS_FILE_SCHEMA_VERSION; + space: typeof ANNOTATIONS_SPACE; + labels: Record>; + tools: { + rulers: WireRuler[]; + rectangles: WireRuler[]; + polygons: WirePolygon[]; + }; +}; + +// The tool and label schemas are strict, so a parsed tool is already exactly the +// allowlisted shape — the only projection left is the one the wire does not do +// for us. Envelope extras survive the schema's passthrough and are dropped by +// the explicit shape returned below. +const withNormalizedPlane = ( + tool: Tool +): Tool => ({ + ...tool, + frameOfReference: { + ...tool.frameOfReference, + planeNormal: normalizePlaneNormal(tool.frameOfReference.planeNormal), + }, +}); + +/** + * Validate an already-JSON-parsed annotations file and project it to the + * allowlisted shape a store may consume. Throws on any structural or semantic + * failure — a dangling label reference, a foreign `schemaVersion`/`space`, or a + * session-only field on a tool — because a partly-understood file must never + * mutate a session. + */ +export const decodeAnnotationsFile = ( + data: unknown +): DecodedAnnotationsFile => { + const parsed = annotationsFileSchema.safeParse(data); + if (!parsed.success) { + const [issue] = parsed.error.issues; + const where = issue?.path?.length ? ` at ${issue.path.join('.')}` : ''; + throw new Error( + `Invalid annotations file${where}: ${issue?.message ?? 'unknown error'}` + ); + } + const file = parsed.data; + return { + schemaVersion: ANNOTATIONS_FILE_SCHEMA_VERSION, + space: ANNOTATIONS_SPACE, + labels: { + rulers: file.labels?.rulers ?? {}, + rectangles: file.labels?.rectangles ?? {}, + polygons: file.labels?.polygons ?? {}, + }, + tools: { + rulers: (file.tools.rulers ?? []).map(withNormalizedPlane), + rectangles: (file.tools.rectangles ?? []).map(withNormalizedPlane), + polygons: (file.tools.polygons ?? []).map(withNormalizedPlane), + }, + }; +}; diff --git a/src/processing/engine/mintAnnotations.ts b/src/processing/engine/mintAnnotations.ts new file mode 100644 index 000000000..72495f191 --- /dev/null +++ b/src/processing/engine/mintAnnotations.ts @@ -0,0 +1,74 @@ +// Bind all finished tools on the active image as one annotations file. The +// image must have server provenance and be declared as a separate task input so +// adopted-job reconstruction can recover it later. + +import type { InputValue } from '@/backend-contract'; +import { TYPE_TAG_ANNOTATIONS } from '@/backend-contract'; +import type { FormValidationIssue, TaskFormModel } from './formModel'; +import type { SourceRefBindingState, SourceRefField } from './mintInput'; +import { ambiguousBinding, sourceRefFields, unboundBinding } from './mintInput'; + +export const annotationsInputFields = ( + model: TaskFormModel +): SourceRefField[] => sourceRefFields(model, TYPE_TAG_ANNOTATIONS); + +export type AnnotationsBindingResult = { + // Parameter ids the caller must stage an annotations file for. + parameters: string[]; + states: Record; + // Caller must suppress its generic issue for these param ids. + issues: FormValidationIssue[]; +}; + +const EMPTY_BINDING: AnnotationsBindingResult = Object.freeze({ + parameters: [], + states: {}, + issues: [], +}); + +const unbound = ( + field: SourceRefField, + state: 'no-annotations' | 'no-provenance' | 'no-reference-input', + // A selected volume that cannot be an input blocks regardless of + // required-ness; anything else only blocks a required field. + alwaysBlocks = false +): AnnotationsBindingResult => ({ + parameters: [], + ...unboundBinding(field, state, 'annotation', alwaysBlocks), +}); + +export const bindAnnotationsInputs = ( + model: TaskFormModel, + hasFinishedTools: boolean, + referenceAvailable: boolean, + declaresReferenceImage: boolean +): AnnotationsBindingResult => { + const fields = annotationsInputFields(model); + if (fields.length === 0) return EMPTY_BINDING; + + // More than one annotations input needs a picker that does not exist. + if (fields.length > 1) { + return { parameters: [], ...ambiguousBinding(fields, 'annotation') }; + } + + const [field] = fields; + + // Checked before the scene states: a task-shape defect cannot be fixed by + // placing tools, so its message must not be masked by theirs. It still leaves + // an optional field submittable — the task simply runs without annotations. + if (!declaresReferenceImage) return unbound(field, 'no-reference-input'); + if (!hasFinishedTools) return unbound(field, 'no-annotations'); + if (!referenceAvailable) return unbound(field, 'no-provenance', true); + + return { + parameters: [field.id], + states: { [field.id]: 'bound' }, + issues: [], + }; +}; + +// `format` is omitted: the staged uri already carries the extension. +export const mintAnnotationsValue = (uris: string[]): InputValue => ({ + type: TYPE_TAG_ANNOTATIONS, + uris, +}); diff --git a/src/processing/engine/mintInput.ts b/src/processing/engine/mintInput.ts index 2c6f627a6..0b108d3f9 100644 --- a/src/processing/engine/mintInput.ts +++ b/src/processing/engine/mintInput.ts @@ -60,8 +60,14 @@ export type SourceRefBindingState = | 'unbound' | 'no-provenance' | 'no-segment-group' + | 'no-annotations' + | 'no-reference-input' | 'ambiguous'; +// What a binder calls the thing it could not bind, in the user-facing +// sentences below. +export type SourceRefNoun = 'image' | 'segment group' | 'annotation'; + export type SourceRefField = Extract; export const sourceRefFields = ( @@ -80,12 +86,16 @@ export const imageInputFields = (model: TaskFormModel): SourceRefField[] => // validation issues and FileWidget renders the same text in the form body. export const bindingStateMessage = ( state: SourceRefBindingState, - noun: 'image' | 'segment group' + 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.'; if (state === 'no-segment-group') return 'Paint or select a segment group first.'; + if (state === 'no-annotations') + return 'Place a ruler, rectangle, or polygon on the current image first.'; + if (state === 'no-reference-input') + return 'This task does not declare the reference image as an input, so its results could not be loaded in a later session.'; if (state === 'ambiguous') return `This task needs more than one ${noun} input, which this version cannot bind automatically.`; return undefined; @@ -95,7 +105,7 @@ export const bindingStateMessage = ( // rather than guess. export const ambiguousBinding = ( fields: SourceRefField[], - noun: 'image' | 'segment group' + noun: SourceRefNoun ): { states: Record; issues: FormValidationIssue[]; @@ -109,6 +119,30 @@ export const ambiguousBinding = ( ], }); +// The other half of every binder's shape: the field could not bind, so it +// carries its state and the sentence explaining it. `alwaysBlocks` separates +// the two reasons — a selected resource that cannot be an input blocks +// regardless of required-ness, while a missing one only blocks a required +// field. +export const unboundBinding = ( + field: SourceRefField, + state: SourceRefBindingState, + noun: SourceRefNoun, + alwaysBlocks = false +): { + states: Record; + issues: FormValidationIssue[]; +} => { + const message = bindingStateMessage(state, noun); + return { + states: { [field.id]: state }, + issues: + message && (alwaysBlocks || field.required) + ? [{ parameter: field.id, message }] + : [], + }; +}; + export type ImageBindingResult = { values: Record; states: Record; @@ -154,13 +188,7 @@ const bindImageFields = ( if (!value) { return { values: nullValues(), - states: { [field.id]: 'no-provenance' }, - issues: [ - { - parameter: field.id, - message: bindingStateMessage('no-provenance', 'image')!, - }, - ], + ...unboundBinding(field, 'no-provenance', 'image', true), }; } diff --git a/src/processing/engine/mintLabelmap.ts b/src/processing/engine/mintLabelmap.ts index ec14ab1de..3f848edfb 100644 --- a/src/processing/engine/mintLabelmap.ts +++ b/src/processing/engine/mintLabelmap.ts @@ -5,9 +5,9 @@ import type { FormValidationIssue, TaskFormModel } from './formModel'; import type { SourceRefBindingState, SourceRefField } from './mintInput'; import { ambiguousBinding, - bindingStateMessage, mintInputValue, sourceRefFields, + unboundBinding, } from './mintInput'; export const labelmapInputFields = (model: TaskFormModel): SourceRefField[] => @@ -83,18 +83,7 @@ const bindLabelmapFields = ( if (resolution.kind === 'unresolved') { return { groups: {}, - states: { [field.id]: 'no-segment-group' }, - issues: field.required - ? [ - { - parameter: field.id, - message: bindingStateMessage( - 'no-segment-group', - 'segment group' - )!, - }, - ] - : [], + ...unboundBinding(field, 'no-segment-group', 'segment group'), }; } diff --git a/src/processing/engine/sourceRefs.ts b/src/processing/engine/sourceRefs.ts index cfc0857d5..b6f765f85 100644 --- a/src/processing/engine/sourceRefs.ts +++ b/src/processing/engine/sourceRefs.ts @@ -1,4 +1,8 @@ -import { TYPE_TAG_IMAGE, TYPE_TAG_LABELMAP } from '@/backend-contract'; +import { + TYPE_TAG_ANNOTATIONS, + TYPE_TAG_IMAGE, + TYPE_TAG_LABELMAP, +} from '@/backend-contract'; import type { DataSource } from '@/src/io/import/dataSource'; import type { TaskFormModel } from './formModel'; import { @@ -15,14 +19,20 @@ import { type LabelmapBindingResult, type SegmentGroupView, } from './mintLabelmap'; +import { + bindAnnotationsInputs, + type AnnotationsBindingResult, +} from './mintAnnotations'; export type BoundSourceRefType = | typeof TYPE_TAG_IMAGE - | typeof TYPE_TAG_LABELMAP; + | typeof TYPE_TAG_LABELMAP + | typeof TYPE_TAG_ANNOTATIONS; export type SourceRefBindings = { image: ImageBindingResult; labelmap: LabelmapBindingResult; + annotations: AnnotationsBindingResult; types: Record; states: Record; issues: ImageBindingResult['issues']; @@ -33,15 +43,22 @@ export type SourceRefBindingContext = { backgroundImageId: string | undefined; activeSegmentGroupId: string | null | undefined; segmentGroups: SegmentGroupView; + // Whether the active image carries at least one finished annotation tool. + hasFinishedAnnotations: boolean; getDataSource: (imageId: string) => DataSource | undefined; }; +const BOUND_TYPES = new Set([ + TYPE_TAG_IMAGE, + TYPE_TAG_LABELMAP, + TYPE_TAG_ANNOTATIONS, +]); + const acceptedTypes = (field: SourceRefField): BoundSourceRefType[] => Array.from( new Set( - field.accepts.filter( - (type): type is BoundSourceRefType => - type === TYPE_TAG_IMAGE || type === TYPE_TAG_LABELMAP + field.accepts.filter((type): type is BoundSourceRefType => + BOUND_TYPES.has(type) ) ) ); @@ -64,15 +81,18 @@ export const bindSourceRefs = ( const fields = model.fields.filter( (field): field is SourceRefField => field.kind === 'sourceRef' ); - const acceptsImage = fields.some((field) => - acceptedTypes(field).includes(TYPE_TAG_IMAGE) - ); - const acceptsLabelmap = fields.some((field) => - acceptedTypes(field).includes(TYPE_TAG_LABELMAP) - ); - const imageValue = acceptsImage - ? mintInputValue(context.activeDataSource, TYPE_TAG_IMAGE) - : null; + const anyFieldAccepts = (type: BoundSourceRefType): boolean => + fields.some((field) => acceptedTypes(field).includes(type)); + + const acceptsImage = anyFieldAccepts(TYPE_TAG_IMAGE); + const acceptsLabelmap = anyFieldAccepts(TYPE_TAG_LABELMAP); + // Annotations stage against the active image itself, so they need the same + // minted value the image binder uses. + const acceptsAnnotations = anyFieldAccepts(TYPE_TAG_ANNOTATIONS); + const imageValue = + acceptsImage || acceptsAnnotations + ? mintInputValue(context.activeDataSource, TYPE_TAG_IMAGE) + : null; const labelmapResolution = acceptsLabelmap ? resolveLabelmapGroup( context.backgroundImageId, @@ -95,24 +115,27 @@ export const bindSourceRefs = ( if (labelmapResolution.kind === 'resolved' && labelmapReference) { available.add(TYPE_TAG_LABELMAP); } + if (context.hasFinishedAnnotations && imageValue) { + available.add(TYPE_TAG_ANNOTATIONS); + } const types: Record = {}; const dedicated = new Set(); fields.forEach((field) => { - const accepted = acceptedTypes(field); - if (accepted.length !== 1) return; - types[field.id] = accepted[0]; - dedicated.add(accepted[0]); + const accepts = acceptedTypes(field); + if (accepts.length !== 1) return; + types[field.id] = accepts[0]; + dedicated.add(accepts[0]); }); fields.forEach((field) => { - const accepted = acceptedTypes(field); - if (accepted.length <= 1) return; - const availableTypes = accepted.filter((type) => available.has(type)); + const accepts = acceptedTypes(field); + if (accepts.length <= 1) return; + const availableTypes = accepts.filter((type) => available.has(type)); const selected = availableTypes.find((type) => !dedicated.has(type)) ?? availableTypes[0] ?? - accepted.find((type) => !dedicated.has(type)) ?? - accepted[0]; + accepts.find((type) => !dedicated.has(type)) ?? + accepts[0]; if (selected) types[field.id] = selected; }); @@ -141,11 +164,21 @@ export const bindSourceRefs = ( }); }); + const annotations = bindAnnotationsInputs( + modelForType(model, types, TYPE_TAG_ANNOTATIONS), + context.hasFinishedAnnotations, + Boolean(imageValue), + // The persisted IMAGE input is what re-identifies the parent after a + // reload; staged types (labelmap, annotations) are excluded there. + Object.values(types).includes(TYPE_TAG_IMAGE) + ); + return { image, labelmap, + annotations, types, - states: { ...image.states, ...labelmap.states }, - issues: [...image.issues, ...labelmapIssues], + states: { ...image.states, ...labelmap.states, ...annotations.states }, + issues: [...image.issues, ...labelmapIssues, ...annotations.issues], }; }; diff --git a/src/processing/store.ts b/src/processing/store.ts index bffbd6898..c1bd7c487 100644 --- a/src/processing/store.ts +++ b/src/processing/store.ts @@ -5,7 +5,11 @@ import { computed, reactive, ref } from 'vue'; import deepEqual from 'fast-deep-equal'; import type { JobHistoryDetail, JobHistorySummary } from '@/backend-contract'; -import { inputValueSchema, TYPE_TAG_LABELMAP } from '@/backend-contract'; +import { + inputValueSchema, + TYPE_TAG_ANNOTATIONS, + TYPE_TAG_LABELMAP, +} from '@/backend-contract'; import { collectProvenanceUris } from '@/src/processing/engine/mintInput'; import type { ProcessingJobStatus, @@ -33,6 +37,14 @@ export const MAX_POLL_RETRIES = 4; export const MAX_POLL_BACKOFF_MS = 30000; export const MAX_JOB_HISTORY_PAGES = 1000; +// Staged inputs derive FROM the scene rather than naming a dataset, so they are +// never parent-image candidates. Excluding them by tag keeps the open image +// vocabulary open: anything else that carries provenance URIs counts. +const STAGED_INPUT_TYPES: ReadonlySet = new Set([ + TYPE_TAG_LABELMAP, + TYPE_TAG_ANNOTATIONS, +]); + const completionReady = (status: ProcessingJobStatus): boolean => isTerminalJobState(status.state); @@ -451,7 +463,7 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { v: unknown ): v is { type: string; uris: string[] } { const parsed = inputValueSchema.safeParse(v); - return parsed.success && parsed.data.type !== TYPE_TAG_LABELMAP; + return parsed.success && !STAGED_INPUT_TYPES.has(parsed.data.type); } // Order-insensitive: a re-loaded dataset's provenance walk need not enumerate diff --git a/src/types/polygon.ts b/src/types/polygon.ts index 8f05d26be..52b3daaac 100644 --- a/src/types/polygon.ts +++ b/src/types/polygon.ts @@ -3,7 +3,7 @@ import { AnnotationTool } from './annotation-tool'; export type Polygon = { /** - * Points is in image index space. + * Points are in world LPS millimeters. */ points: Array; } & AnnotationTool; diff --git a/src/types/ruler.ts b/src/types/ruler.ts index 231e98f9a..4f710690a 100644 --- a/src/types/ruler.ts +++ b/src/types/ruler.ts @@ -3,11 +3,11 @@ import { AnnotationTool } from './annotation-tool'; export type Ruler = { /** - * Point is in image index space. + * Point is in world LPS millimeters. */ firstPoint: Vector3; /** - * Point is in image index space. + * Point is in world LPS millimeters. */ secondPoint: Vector3; } & AnnotationTool; diff --git a/src/utils/bugReport.ts b/src/utils/bugReport.ts index f14d6715a..59181ba62 100644 --- a/src/utils/bugReport.ts +++ b/src/utils/bugReport.ts @@ -4,11 +4,10 @@ import { useDatasetStore } from '@/src/store/datasets'; import { useDICOMStore } from '@/src/store/datasets-dicom'; import { useImageCacheStore } from '@/src/store/image-cache'; import { useSegmentGroupStore } from '@/src/store/segmentGroups'; +import { COMPOUND_EXTENSIONS } from '@/src/utils/path'; const MAX_ERROR_LENGTH = 4000; -const COMPOUND_EXTENSIONS = ['nii.gz', 'iwi.cbor', 'seg.nrrd']; - const getBrowserInfo = (): string => typeof navigator !== 'undefined' ? navigator.userAgent : 'unknown'; diff --git a/src/utils/index.ts b/src/utils/index.ts index c722704f3..102161c72 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -223,13 +223,10 @@ export function getErrorDetail(error: unknown, fallback: string): string { } // remove undefined properties -export function cleanUndefined(obj: Object) { - return Object.entries(obj).reduce( - (cleaned, [key, value]) => - value === undefined ? cleaned : { ...cleaned, [key]: value }, - {} - ); -} +export const cleanUndefined = (record: T): Partial => + Object.fromEntries( + Object.entries(record).filter(([, value]) => value !== undefined) + ) as Partial; // converts named colors (red, antiquewhite, etc) to hex export function standardizeColor(color: Maybe) { diff --git a/src/utils/path.ts b/src/utils/path.ts index f1f8307c3..6db2fa254 100644 --- a/src/utils/path.ts +++ b/src/utils/path.ts @@ -21,6 +21,28 @@ export function basename(path: string) { return path.split(/[\\/]+/g).at(-1) ?? path; } +/** + * Extensions whose meaning spans more than one dotted segment. + */ +export const COMPOUND_EXTENSIONS = ['nii.gz', 'iwi.cbor', 'seg.nrrd']; + +/** + * Returns the base name of a path without its extension. + * + * Compound-aware, so "scan.nii.gz" yields "scan" rather than "scan.nii". A + * leading dot is kept: ".nrrd" is a name, not an extension. + * @param path + * @returns + */ +export function stripExtension(path: string) { + const base = basename(path); + const lower = base.toLowerCase(); + const compound = COMPOUND_EXTENSIONS.find((ext) => lower.endsWith(`.${ext}`)); + if (compound) return base.slice(0, -(compound.length + 1)); + const dot = base.lastIndexOf('.'); + return dot > 0 ? base.slice(0, dot) : base; +} + /** * Normalizes a string. * From 25eca477045c95ed09029779b072a0eeeee32282 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 27 Jul 2026 20:51:12 -0400 Subject: [PATCH 3/4] feat(processing): apply vector annotations from job results Add the add-annotations result intent to the neutral contract and bump the intent vocabulary. Preflight each result, then merge its labels and tools as one atomic update. Resolve tool planes and advisory frames against the target image without changing the active label. Persist structured result provenance for idempotency across session restores. --- .../fixtures/wire/intent.add-annotations.json | 11 + .../generated/job-results.schema.json | 67 ++ backend-contract/generated/openapi.json | 69 +- .../generated/result-intent.schema.json | 67 ++ .../processing/__tests__/openapi.spec.ts | 2 +- .../processing/__tests__/wire.spec.ts | 54 +- backend-contract/processing/openapi.ts | 5 +- backend-contract/processing/wire.ts | 24 +- .../__tests__/annotationToolSource.spec.ts | 131 ++++ .../__tests__/segmentGroupSource.spec.ts | 44 +- src/io/state-file/migrations.ts | 8 +- src/io/state-file/schema.ts | 17 +- .../applyResults.annotations.spec.ts | 591 ++++++++++++++++++ src/processing/__tests__/applyResults.spec.ts | 18 +- src/processing/applyResults.ts | 281 ++++++++- .../annotationToolImageDelete.spec.ts | 1 + src/store/__tests__/rulers.spec.ts | 1 + src/store/segmentGroups.ts | 13 +- src/store/tools/useLabels.ts | 7 +- src/types/annotation-tool.ts | 4 + src/types/index.ts | 14 + 21 files changed, 1352 insertions(+), 77 deletions(-) create mode 100644 backend-contract/fixtures/wire/intent.add-annotations.json create mode 100644 src/io/state-file/__tests__/annotationToolSource.spec.ts create mode 100644 src/processing/__tests__/applyResults.annotations.spec.ts diff --git a/backend-contract/fixtures/wire/intent.add-annotations.json b/backend-contract/fixtures/wire/intent.add-annotations.json new file mode 100644 index 000000000..1900c9a03 --- /dev/null +++ b/backend-contract/fixtures/wire/intent.add-annotations.json @@ -0,0 +1,11 @@ +{ + "id": "6600000000000000000000e4", + "intent": "add-annotations", + "url": "/api/v1/file/6600000000000000000000e4/proxiable/rois.annotations.json", + "name": "rois.annotations.json", + "source": { + "providerId": "analysis-provider", + "jobId": "job-abc123", + "outputId": "outputAnnotations" + } +} diff --git a/backend-contract/generated/job-results.schema.json b/backend-contract/generated/job-results.schema.json index ce9a41e5c..79b8f1c1f 100644 --- a/backend-contract/generated/job-results.schema.json +++ b/backend-contract/generated/job-results.schema.json @@ -227,6 +227,73 @@ "url" ], "additionalProperties": {} + }, + { + "type": "object", + "properties": { + "intent": { + "type": "string", + "const": "add-annotations" + }, + "id": { + "type": "string", + "minLength": 1 + }, + "name": { + "type": "string" + }, + "url": { + "type": "string" + }, + "mimeType": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, + "size": { + "anyOf": [ + { + "type": "number", + "minimum": 0 + }, + { + "type": "null" + } + ] + }, + "source": { + "type": "object", + "properties": { + "providerId": { + "type": "string" + }, + "jobId": { + "type": "string" + }, + "outputId": { + "type": "string" + } + }, + "required": [ + "providerId", + "jobId", + "outputId" + ], + "additionalProperties": false + } + }, + "required": [ + "intent", + "id", + "name", + "url" + ], + "additionalProperties": {} } ] }, diff --git a/backend-contract/generated/openapi.json b/backend-contract/generated/openapi.json index 5db5e18b0..3b922115c 100644 --- a/backend-contract/generated/openapi.json +++ b/backend-contract/generated/openapi.json @@ -4,7 +4,7 @@ "info": { "title": "VolView neutral backend contract", "version": "0.1.0", - "description": "DRAFT 0.x — shapes may change until a second backend passes the conformance kit (the pinned 1.0 criterion). The neutral REST surface the VolView client calls to run processing tasks against a backend. A conforming server-side BACKEND implements these endpoints and the referenced wire schemas — no VolView client change is needed to bring a new backend online. Everything here is neutral: no backend routes, ids, status enums, or URL shapes leak. The artifact version is the draft artifact version, distinct from the shape versions: the result-intent vocabulary is at version 1 (INTENT_VOCABULARY_VERSION); the task-spec shape at version 1 (specVersion)." + "description": "DRAFT 0.x — shapes may change until a second backend passes the conformance kit (the pinned 1.0 criterion). The neutral REST surface the VolView client calls to run processing tasks against a backend. A conforming server-side BACKEND implements these endpoints and the referenced wire schemas — no VolView client change is needed to bring a new backend online. Everything here is neutral: no backend routes, ids, status enums, or URL shapes leak. The artifact version is the draft artifact version, distinct from the shape versions: the result-intent vocabulary is at version 2 (INTENT_VOCABULARY_VERSION); the task-spec shape at version 1 (specVersion)." }, "servers": [ { @@ -1312,6 +1312,73 @@ "url" ], "additionalProperties": {} + }, + { + "type": "object", + "properties": { + "intent": { + "type": "string", + "const": "add-annotations" + }, + "id": { + "type": "string", + "minLength": 1 + }, + "name": { + "type": "string" + }, + "url": { + "type": "string" + }, + "mimeType": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, + "size": { + "anyOf": [ + { + "type": "number", + "minimum": 0 + }, + { + "type": "null" + } + ] + }, + "source": { + "type": "object", + "properties": { + "providerId": { + "type": "string" + }, + "jobId": { + "type": "string" + }, + "outputId": { + "type": "string" + } + }, + "required": [ + "providerId", + "jobId", + "outputId" + ], + "additionalProperties": false + } + }, + "required": [ + "intent", + "id", + "name", + "url" + ], + "additionalProperties": {} } ] }, diff --git a/backend-contract/generated/result-intent.schema.json b/backend-contract/generated/result-intent.schema.json index 9eeba14a4..e0b27e58b 100644 --- a/backend-contract/generated/result-intent.schema.json +++ b/backend-contract/generated/result-intent.schema.json @@ -215,6 +215,73 @@ "url" ], "additionalProperties": {} + }, + { + "type": "object", + "properties": { + "intent": { + "type": "string", + "const": "add-annotations" + }, + "id": { + "type": "string", + "minLength": 1 + }, + "name": { + "type": "string" + }, + "url": { + "type": "string" + }, + "mimeType": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, + "size": { + "anyOf": [ + { + "type": "number", + "minimum": 0 + }, + { + "type": "null" + } + ] + }, + "source": { + "type": "object", + "properties": { + "providerId": { + "type": "string" + }, + "jobId": { + "type": "string" + }, + "outputId": { + "type": "string" + } + }, + "required": [ + "providerId", + "jobId", + "outputId" + ], + "additionalProperties": false + } + }, + "required": [ + "intent", + "id", + "name", + "url" + ], + "additionalProperties": {} } ] }, diff --git a/backend-contract/processing/__tests__/openapi.spec.ts b/backend-contract/processing/__tests__/openapi.spec.ts index 86ba49ccb..b4f652bf3 100644 --- a/backend-contract/processing/__tests__/openapi.spec.ts +++ b/backend-contract/processing/__tests__/openapi.spec.ts @@ -140,7 +140,7 @@ describe('single source — published shapes track the source of truth', () => { expect(states).toEqual([...JOB_STATES]); }); - it('publishes the full v1 result-intent vocabulary', () => { + it('publishes the full result-intent vocabulary', () => { const serialized = JSON.stringify(schemaComponents().ResultIntent); RESULT_INTENTS.forEach((intent) => { expect(serialized).toContain(intent); diff --git a/backend-contract/processing/__tests__/wire.spec.ts b/backend-contract/processing/__tests__/wire.spec.ts index 2ee69a069..603c1bed6 100644 --- a/backend-contract/processing/__tests__/wire.spec.ts +++ b/backend-contract/processing/__tests__/wire.spec.ts @@ -194,12 +194,13 @@ describe('neutral job status fixtures', () => { // --------------------------------------------------------------------------- describe('result intent fixtures', () => { - it('exports vocabulary version 1 and the exactly-three state intents', () => { - expect(INTENT_VOCABULARY_VERSION).toBe(1); + it('exports vocabulary version 2 and the exactly-four state intents', () => { + expect(INTENT_VOCABULARY_VERSION).toBe(2); expect([...RESULT_INTENTS]).toEqual([ 'add-base-image', 'add-layer', 'add-segment-group', + 'add-annotations', ]); expect(wire).not.toHaveProperty('intent.download'); }); @@ -209,11 +210,49 @@ describe('result intent fixtures', () => { 'intent.add-layer', 'intent.add-segment-group.with-segments', 'intent.add-segment-group.embedded', + 'intent.add-annotations', 'intent.unknown', ])('validates %s', (name) => { expect(() => resultIntentSchema.parse(wire[name])).not.toThrow(); }); + it('parses add-annotations as a KNOWN intent carrying a source tag', () => { + const fixture = wire['intent.add-annotations']; + expect(knownResultIntentSchema.safeParse(fixture).success).toBe(true); + const parsed = resultIntentSchema.parse(fixture) as Record; + expect(parsed.intent).toBe('add-annotations'); + expect(parsed.source).toEqual({ + providerId: 'analysis-provider', + jobId: 'job-abc123', + outputId: 'outputAnnotations', + }); + // Labels ride inside the annotations file, so there is no `segments` peer. + expect(parsed).not.toHaveProperty('segments'); + }); + + it('accepts add-annotations without a source (source is optional)', () => { + expect( + knownResultIntentSchema.safeParse({ + id: 'r1', + intent: 'add-annotations', + url: '/rois.annotations.json', + name: 'rois.annotations.json', + }).success + ).toBe(true); + }); + + it('rejects an add-annotations source missing provider identity', () => { + expect( + knownResultIntentSchema.safeParse({ + id: 'r1', + intent: 'add-annotations', + url: '/rois.annotations.json', + name: 'rois.annotations.json', + source: { jobId: 'job-abc123', outputId: 'outputAnnotations' }, + }).success + ).toBe(false); + }); + it('parses add-segment-group WITH segments and a source provenance tag', () => { const parsed = resultIntentSchema.parse( wire['intent.add-segment-group.with-segments'] @@ -257,6 +296,17 @@ describe('result intent fixtures', () => { expect(parsed.name).toBeTruthy(); }); + it('keeps the unknown-intent fixture unknown after the vocabulary grew', () => { + // `add-polygon` is deliberately NOT a member of the vocabulary: it is the + // pinned fail-open example, and growing the vocabulary must not quietly + // adopt it. Adding an intent is exactly the kind of change that could. + const fixture = wire['intent.unknown'] as { intent: string }; + expect(fixture.intent).toBe('add-polygon'); + expect(RESULT_INTENTS).not.toContain(fixture.intent); + expect(knownResultIntentSchema.safeParse(fixture).success).toBe(false); + expect(resultIntentSchema.safeParse(fixture).success).toBe(true); + }); + it.each([ ['missing', { id: 'r1', url: '/report.csv', name: 'report.csv' }], [ diff --git a/backend-contract/processing/openapi.ts b/backend-contract/processing/openapi.ts index 21b5d31e1..413ef453a 100644 --- a/backend-contract/processing/openapi.ts +++ b/backend-contract/processing/openapi.ts @@ -44,8 +44,9 @@ const WIRE_COMPONENTS: Record = { 'job-results': 'JobResults', 'job-results-error': 'JobResultsError', // Not a request/response body: the interchange FILE format a staged - // `annotations` input carries. Published as a component so a backend author - // sees the whole vocabulary, including the file formats, in one document. + // `annotations` input carries and an `add-annotations` result points at. + // Published as a component so a backend author sees the whole vocabulary, + // including the file formats, in one document. 'annotations-file': 'AnnotationsFile', }; diff --git a/backend-contract/processing/wire.ts b/backend-contract/processing/wire.ts index 1eacb91d8..27c2d41ba 100644 --- a/backend-contract/processing/wire.ts +++ b/backend-contract/processing/wire.ts @@ -19,8 +19,9 @@ import { import { pathSegmentIdSchema } from './ids'; // Bump when the intent vocabulary's shape changes so producers and the applier -// can negotiate compatibility. -export const INTENT_VOCABULARY_VERSION = 1; +// can negotiate compatibility. Adding an intent is a compatible bump: an older +// client demotes the unknown intent through the fail-open branch above. +export const INTENT_VOCABULARY_VERSION = 2; // --------------------------------------------------------------------------- // Input value: what the client sends at submit @@ -158,12 +159,12 @@ export const RESULT_INTENTS = [ 'add-base-image', 'add-layer', 'add-segment-group', + 'add-annotations', ] as const; export type ResultIntentName = (typeof RESULT_INTENTS)[number]; -// Provenance tag stamped on an applied segment group: an idempotency key. -// Structurally identical to the `source?` field on `SegmentGroupMetadata` so it -// round-trips the `.volview.zip`. +// Provenance tag on a result: the durable idempotency identity the client +// preserves on generated scene state so restored results can be recognized. export const resultSourceSchema = z.object({ providerId: z.string(), jobId: z.string(), @@ -224,6 +225,18 @@ const addSegmentGroup = z }) .passthrough(); +// `add-annotations` points at an `annotations.ts` interchange file: the vector +// annotations to add to the referenced image. It carries NO `segments` +// equivalent — the label namespaces ride inside the file itself — plus the same +// optional `source` provenance tag used as the idempotency key. +const addAnnotations = z + .object({ + intent: z.literal('add-annotations'), + ...resultListItemSchema.shape, + source: resultSourceSchema.optional(), + }) + .passthrough(); + // The STRICT half of the vocabulary: every declared state directive with its // declared shape. Exported so the single applier can gate on which union member // strictly matched — a name-known-but-shape-invalid result (e.g. a broken @@ -232,6 +245,7 @@ export const knownResultIntentSchema = z.discriminatedUnion('intent', [ addBaseImage, addLayer, addSegmentGroup, + addAnnotations, ]); export type KnownResultIntent = z.infer; diff --git a/src/io/state-file/__tests__/annotationToolSource.spec.ts b/src/io/state-file/__tests__/annotationToolSource.spec.ts new file mode 100644 index 000000000..1f3c2e82b --- /dev/null +++ b/src/io/state-file/__tests__/annotationToolSource.spec.ts @@ -0,0 +1,131 @@ +import { describe, expect, it, beforeEach } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; + +import { ManifestSchema } from '@/src/io/state-file/schema'; +import { migrateManifest } from '@/src/io/state-file/migrations'; +import { MANIFEST_VERSION } from '@/src/io/state-file/serialize'; +import { useRulerStore } from '@/src/store/tools/rulers'; + +// --------------------------------------------------------------------------- +// The optional structured `source` on an annotation tool is the durable +// idempotency identity that stops a restored job result from being applied twice. +// It only works if it survives the `.volview.zip` — and the manifest schema +// strips unknown keys on parse, so "does it round-trip" is the whole test. +// --------------------------------------------------------------------------- + +const source = { + providerId: 'analysis-provider', + jobId: 'job-abc', + outputId: 'outputAnnotations', +}; + +const ruler = (extra: Record = {}) => ({ + imageID: 'img-1', + frameOfReference: { planeOrigin: [0, 0, 5], planeNormal: [0, 0, 1] }, + slice: 5, + firstPoint: [1, 1, 5], + secondPoint: [4, 4, 5], + name: 'Long axis', + ...extra, +}); + +const manifestWith = (tools: Record) => ({ + version: MANIFEST_VERSION, + dataSources: [], + tools, +}); + +describe('annotation tool source', () => { + it('round-trips a source through a full manifest parse', () => { + const parsed = ManifestSchema.parse( + manifestWith({ + rulers: { tools: [ruler({ source })], labels: {} }, + }) + ); + expect(parsed.tools?.rulers?.tools[0].source).toEqual(source); + }); + + it('round-trips on rectangles and polygons too', () => { + const parsed = ManifestSchema.parse( + manifestWith({ + rectangles: { tools: [ruler({ source })], labels: {} }, + polygons: { + tools: [ + { + imageID: 'img-1', + frameOfReference: { + planeOrigin: [0, 0, 3], + planeNormal: [0, 0, 1], + }, + slice: 3, + points: [ + [1, 1, 3], + [5, 1, 3], + [3, 5, 3], + ], + source, + }, + ], + labels: {}, + }, + }) + ); + expect(parsed.tools?.rectangles?.tools[0].source).toEqual(source); + expect(parsed.tools?.polygons?.tools[0].source).toEqual(source); + }); + + it('is optional — a hand-placed tool has none', () => { + const parsed = ManifestSchema.parse( + manifestWith({ rulers: { tools: [ruler()], labels: {} } }) + ); + expect(parsed.tools?.rulers?.tools[0].source).toBeUndefined(); + }); + + it('rejects a source missing one identity component', () => { + const bad = manifestWith({ + rulers: { + tools: [ruler({ source: { providerId: 'p', jobId: 'j' } })], + labels: {}, + }, + }); + expect(ManifestSchema.safeParse(bad).success).toBe(false); + }); + + // The annotation `source` field is additive-optional, so 6.4.0 remains the + // current manifest version and passes through untouched. + it('passes a 6.4.0 manifest without touching its tools', () => { + const old = JSON.stringify({ + version: '6.4.0', + dataSources: [], + tools: { rulers: { tools: [ruler()], labels: {} } }, + }); + const migrated = migrateManifest(old); + expect(migrated.version).toBe(MANIFEST_VERSION); + expect(() => ManifestSchema.parse(migrated)).not.toThrow(); + expect(migrated.tools.rulers.tools[0]).toEqual(ruler()); + }); +}); + +describe('annotation tool source — store serialize/restore', () => { + beforeEach(() => { + setActivePinia(createPinia()); + }); + + it('survives serializeTools -> manifest parse -> deserializeTools', () => { + const store = useRulerStore(); + store.addTool({ ...ruler({ source, placing: false }) } as never); + + const serialized = store.serializeTools(); + const parsed = ManifestSchema.parse(manifestWith({ rulers: serialized })) + .tools!.rulers!; + expect(parsed.tools[0].source).toEqual(source); + + setActivePinia(createPinia()); + const restored = useRulerStore(); + restored.deserializeTools(parsed as never, { 'img-1': 'img-2' }); + + const [id] = restored.toolIDs; + expect(restored.toolByID[id].source).toEqual(source); + expect(restored.toolByID[id].imageID).toBe('img-2'); + }); +}); diff --git a/src/io/state-file/__tests__/segmentGroupSource.spec.ts b/src/io/state-file/__tests__/segmentGroupSource.spec.ts index d13e93bc7..38771e534 100644 --- a/src/io/state-file/__tests__/segmentGroupSource.spec.ts +++ b/src/io/state-file/__tests__/segmentGroupSource.spec.ts @@ -7,9 +7,8 @@ import { import { migrateManifest } from '@/src/io/state-file/migrations'; import { MANIFEST_VERSION } from '@/src/io/state-file/serialize'; -// The optional `source: {providerId, jobId, outputId}` provenance tag on -// SegmentGroupMetadata — the durable job-history idempotency key that must round-trip the -// `.volview.zip`. +// The optional structured `source` on SegmentGroupMetadata is the durable +// idempotency identity that must round-trip the `.volview.zip`. const baseMetadata = { name: 'Otsu result', @@ -32,13 +31,9 @@ const metadataWithSource = { }; describe('SegmentGroupMetadata.source', () => { - it('accepts and round-trips a source provenance tag', () => { + it('accepts and round-trips structured provenance', () => { const parsed = SegmentGroupMetadata.parse(metadataWithSource); - expect(parsed.source).toEqual({ - providerId: 'analysis-provider', - jobId: 'job-abc', - outputId: 'outputLabelmap', - }); + expect(parsed.source).toEqual(metadataWithSource.source); }); it('is optional — a hand-painted group without source still validates', () => { @@ -46,18 +41,13 @@ describe('SegmentGroupMetadata.source', () => { expect(SegmentGroupMetadata.parse(baseMetadata).source).toBeUndefined(); }); - it('rejects a malformed source (missing outputId)', () => { - const bad = { - ...metadataWithSource, - source: { providerId: 'analysis-provider', jobId: 'job-abc' }, - }; - expect(SegmentGroupMetadata.safeParse(bad).success).toBe(false); - }); - - it('rejects a source without provider identity', () => { + it('rejects a source missing one identity component', () => { const bad = { ...metadataWithSource, - source: { jobId: 'job-abc', outputId: 'outputLabelmap' }, + source: { + providerId: 'analysis-provider', + jobId: 'job-abc', + }, }; expect(SegmentGroupMetadata.safeParse(bad).success).toBe(false); }); @@ -71,20 +61,20 @@ describe('SegmentGroupMetadata.source', () => { ], }; const parsed = ManifestSchema.parse(manifest); - expect(parsed.segmentGroups?.[0].metadata.source).toEqual({ - providerId: 'analysis-provider', - jobId: 'job-abc', - outputId: 'outputLabelmap', - }); + expect(parsed.segmentGroups?.[0].metadata.source).toEqual( + metadataWithSource.source + ); }); }); describe('manifest version / migration bump', () => { - it('pins MANIFEST_VERSION at 6.4.0', () => { + // Annotation provenance is additive to the structured segment-group source + // already covered by 6.4.0, so it needs no stamp-only version bump. + it('keeps MANIFEST_VERSION at 6.4.0', () => { expect(MANIFEST_VERSION).toBe('6.4.0'); }); - it('migrates a 6.3.0 manifest to 6.4.0, preserving segment groups', () => { + it('migrates a 6.3.0 manifest to the current version, preserving segment groups', () => { const old = JSON.stringify({ version: '6.3.0', dataSources: [], @@ -101,7 +91,7 @@ describe('manifest version / migration bump', () => { ], }); const migrated = migrateManifest(old); - expect(migrated.version).toBe('6.4.0'); + expect(migrated.version).toBe(MANIFEST_VERSION); expect(migrated.segmentGroups).toHaveLength(1); // An old manifest lacking `source` still validates (additive-optional). expect(() => ManifestSchema.parse(migrated)).not.toThrow(); diff --git a/src/io/state-file/migrations.ts b/src/io/state-file/migrations.ts index 503342e7b..12588de17 100644 --- a/src/io/state-file/migrations.ts +++ b/src/io/state-file/migrations.ts @@ -155,10 +155,10 @@ const migrate610To620 = (inputManifest: any) => { }; // 6.3.0 -> 6.4.0 adds the optional `source` provenance tag to segment-group -// metadata. The field is additive-optional, so an older manifest that -// lacks it still validates — the bump only stamps the version (no data -// transform). No 6.2.0 -> 6.3.0 step exists: a 6.2 manifest validates -// unmodified. +// metadata and to annotation tools (rulers, rectangles, polygons). The field is +// additive-optional, so an older manifest that lacks it still validates — the +// bump only stamps the version (no data transform). No 6.2.0 -> 6.3.0 step +// exists: a 6.2 manifest validates unmodified. const migrate630To640 = (inputManifest: any) => ({ ...inputManifest, version: '6.4.0', diff --git a/src/io/state-file/schema.ts b/src/io/state-file/schema.ts index 7fc57a655..211168fa0 100644 --- a/src/io/state-file/schema.ts +++ b/src/io/state-file/schema.ts @@ -308,12 +308,11 @@ const SegmentMask = z.object({ locked: z.boolean().optional(), }); -// Provenance of a segment group produced by a processing job. This durable -// idempotency key prevents a restored result from being applied twice. -// Optional and additive — a hand-painted group has none. Round-trips the -// `.volview.zip` as interchange. Structurally mirrors the backend-contract -// `resultSource` wire tag. -export const SegmentGroupSource = z.object({ +// Provenance of a scene object produced by a processing job. This durable +// identity prevents a restored result from being applied twice. Optional and +// additive wherever it is used; hand-made state has none. The shape mirrors the +// backend contract's result source and is shared by groups and annotation tools. +export const ProcessingResultSource = z.object({ providerId: z.string(), jobId: z.string(), outputId: z.string(), @@ -335,7 +334,7 @@ export const SegmentGroupMetadata = z.object({ byValue: z.record(z.string(), SegmentMask), }) .optional(), - source: SegmentGroupSource.optional(), + source: ProcessingResultSource.optional(), }); export const SegmentGroup = z @@ -377,6 +376,10 @@ const annotationTool = z.object({ label: z.string().optional(), labelName: z.string().optional(), metadata: z.record(z.string(), z.string()).optional(), + // Job provenance, present only on a tool applied from a result. Unknown keys + // are stripped on parse, so restore would silently drop the idempotency key + // without this declaration. + source: ProcessingResultSource.optional(), }); const makeToolEntry = (tool: z.ZodObject) => diff --git a/src/processing/__tests__/applyResults.annotations.spec.ts b/src/processing/__tests__/applyResults.annotations.spec.ts new file mode 100644 index 000000000..468510f91 --- /dev/null +++ b/src/processing/__tests__/applyResults.annotations.spec.ts @@ -0,0 +1,591 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; +import { nextTick } from 'vue'; +import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; +import vtkDataArray from '@kitware/vtk.js/Common/Core/DataArray'; + +import { applyIntent } from '@/src/processing/applyResults'; +import type { SubmittedJobContext } from '@/src/processing/types'; +import { useImageCacheStore } from '@/src/store/image-cache'; +import { useDICOMStore } from '@/src/store/datasets-dicom'; +import { useRulerStore } from '@/src/store/tools/rulers'; +import { useRectangleStore } from '@/src/store/tools/rectangles'; +import { usePolygonStore } from '@/src/store/tools/polygons'; + +// --------------------------------------------------------------------------- +// Applying an `add-annotations` result. +// +// The stores are REAL here: the contract this exercises is what actually lands +// in a session — the derived slice, the label ids `addTool` re-reads styles +// from, and the durable `source` receipt — none of which a store double could +// tell the truth about. Only the heavy import/download edges are mocked. +// --------------------------------------------------------------------------- + +const mocks = vi.hoisted(() => ({ + fetchProcessingResult: vi.fn(), +})); + +vi.mock('@/src/processing/engine/resultDownload', () => ({ + fetchProcessingResult: mocks.fetchProcessingResult, +})); +vi.mock('@/src/io/import/dataSource', () => ({ uriToDataSource: vi.fn() })); +vi.mock('@/src/io/import/importDataSources', () => ({ + importVolumeDataSources: vi.fn(), + toDataSelection: vi.fn(), +})); +vi.mock('@/src/io/import/common', () => ({ isVolumeResult: vi.fn() })); +vi.mock('@/src/actions/loadUserFiles', () => ({ loadVolumeUrls: vi.fn() })); + +const IMAGE_ID = 'img-1'; + +// A 20mm cube at the origin with unit spacing: world LPS mm and image indices +// coincide, so a plane origin's z IS its slice. +function seatImage(id = IMAGE_ID) { + const image = vtkImageData.newInstance(); + image.setDimensions(20, 20, 20); + image.getPointData().setScalars( + vtkDataArray.newInstance({ + name: 'scalars', + numberOfComponents: 1, + values: new Uint8Array(20 * 20 * 20), + }) + ); + return useImageCacheStore().addVTKImageData(image, 'CT', { id }); +} + +const axialAt = (z: number) => ({ + planeNormal: [0, 0, 1], + planeOrigin: [0, 0, z], +}); + +const context = (activeDatasetId?: string): SubmittedJobContext => ({ + jobId: 'job-1', + taskId: 'task-1', + providerId: 'provider-1', + submittedAt: '2026-07-27T00:00:00Z', + activeDatasetId, +}); + +const source = { + providerId: 'provider-1', + jobId: 'job-1', + outputId: 'outputAnnotations', +}; + +const intent = (overrides: Record = {}) => + ({ + intent: 'add-annotations', + id: 'r1', + name: 'out.annotations.json', + url: 'https://example/out.annotations.json', + source, + ...overrides, + }) as never; + +// Wire files are hand-written rather than encoded from a view: this is the +// producer's half of the boundary, and a task is not VolView. Typed loosely on +// purpose so a test can bend one field into something a producer might emit. +type WireLabels = Record< + string, + { color?: string; strokeWidth?: number; fillColor?: string } +>; +type WireTool = Record; +type WireFile = { + schemaVersion: unknown; + space: unknown; + labels: { rulers: WireLabels; rectangles: WireLabels; polygons: WireLabels }; + tools: { rulers: WireTool[]; rectangles: WireTool[]; polygons: WireTool[] }; +}; + +const annotationsFile = (): WireFile => ({ + schemaVersion: 1, + space: 'LPS', + labels: { + // The SAME name in two namespaces with different styles — legal, because + // the stores are independent. + rulers: { roi: { color: '#ff0000', strokeWidth: 3 } }, + rectangles: { roi: { color: '#00ff00', fillColor: '#00ff0033' } }, + polygons: { lesion: { color: '#0000ff' } }, + }, + tools: { + rulers: [ + { + firstPoint: [1, 1, 5], + secondPoint: [4, 4, 5], + frameOfReference: axialAt(5), + labelName: 'roi', + name: 'Long axis', + // Advisory only, and deliberately a lie: the applier re-derives 5. + slice: 99, + metadata: { origin: 'RulerToRectangle' }, + }, + ], + rectangles: [ + { + firstPoint: [2, 2, 7], + secondPoint: [6, 6, 7], + frameOfReference: axialAt(7), + labelName: 'roi', + }, + ], + polygons: [ + { + points: [ + [1, 1, 3], + [5, 1, 3], + [3, 5, 3], + ], + frameOfReference: axialAt(3), + labelName: 'lesion', + }, + ], + }, +}); + +const serveFile = (body: unknown) => { + const text = typeof body === 'string' ? body : JSON.stringify(body); + mocks.fetchProcessingResult.mockResolvedValue( + new File([text], 'out.annotations.json', { type: 'application/json' }) + ); +}; + +const toolCounts = () => ({ + rulers: useRulerStore().toolIDs.length, + rectangles: useRectangleStore().toolIDs.length, + polygons: usePolygonStore().toolIDs.length, +}); + +const onlyTool = (store: { + toolIDs: string[]; + toolByID: Record; +}) => store.toolByID[store.toolIDs[0]]; + +beforeEach(() => { + vi.clearAllMocks(); + setActivePinia(createPinia()); + seatImage(); + serveFile(annotationsFile()); +}); + +describe('applyIntent — add-annotations', () => { + it('adds every tool kind to the job image, deriving the slice from the frame', async () => { + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('applied'); + expect(toolCounts()).toEqual({ rulers: 1, rectangles: 1, polygons: 1 }); + + const ruler = onlyTool(useRulerStore()); + expect(ruler.imageID).toBe(IMAGE_ID); + // The wire said 99; the frame of reference says 5, and it wins. + expect(ruler.slice).toBe(5); + expect(ruler.firstPoint).toEqual([1, 1, 5]); + expect(ruler.secondPoint).toEqual([4, 4, 5]); + expect(ruler.name).toBe('Long axis'); + expect(ruler.metadata).toEqual({ origin: 'RulerToRectangle' }); + expect(ruler.placing).toBe(false); + // The idempotency receipt is durable session state. + expect(ruler.source).toEqual(source); + + expect(onlyTool(useRectangleStore()).slice).toBe(7); + expect(onlyTool(usePolygonStore())).toMatchObject({ + slice: 3, + imageID: IMAGE_ID, + points: [ + [1, 1, 3], + [5, 1, 3], + [3, 5, 3], + ], + }); + }); + + it('applies native rectangles on a rotated acquisition without inventing a basis', async () => { + const half = Math.sqrt(0.5); + const image = vtkImageData.newInstance(); + image.setDimensions(20, 20, 20); + image.setDirection([half, half, 0, -half, half, 0, 0, 0, 1]); + image.getPointData().setScalars( + vtkDataArray.newInstance({ + name: 'scalars', + numberOfComponents: 1, + values: new Uint8Array(20 * 20 * 20), + }) + ); + const imageID = 'rotated-image'; + useImageCacheStore().addVTKImageData(image, 'CT', { id: imageID }); + await nextTick(); + + const file = annotationsFile(); + file.tools.rulers = []; + file.tools.polygons = []; + file.tools.rectangles = [ + { + firstPoint: [-2, 2, 7], + secondPoint: [2, 6, 7], + frameOfReference: axialAt(7), + labelName: 'roi', + }, + ]; + serveFile(file); + + const outcome = await applyIntent(intent(), context(imageID)); + + expect( + outcome.status, + String((outcome as { error?: Error }).error ?? '') + ).toBe('applied'); + expect(onlyTool(useRectangleStore())).toMatchObject({ + imageID, + firstPoint: [-2, 2, 7], + secondPoint: [2, 6, 7], + slice: 7, + }); + }); + + it.each([ + [ + [0, 0, 2], + [0, 0, 1], + ], + [ + [0, 0, -3], + [0, 0, -1], + ], + [ + [0, 0, 0.99999], + [0, 0, 1], + ], + ])( + 'normalizes plane normal %j before axis matching and storage', + async (planeNormal, expected) => { + const file = annotationsFile(); + file.tools.rulers[0].frameOfReference = { + planeNormal, + planeOrigin: [0, 0, 5], + }; + serveFile(file); + + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + + expect(outcome.status).toBe('applied'); + expect(onlyTool(useRulerStore()).frameOfReference.planeNormal).toEqual( + expected + ); + } + ); + + it('rejects a zero plane normal before mutating any store', async () => { + const file = annotationsFile(); + file.tools.polygons[0].frameOfReference = { + planeNormal: [0, 0, 0], + planeOrigin: [0, 0, 3], + }; + serveFile(file); + + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + + expect(outcome.status).toBe('failed'); + expect(String((outcome as { error: Error }).error)).toContain( + 'nonzero vector' + ); + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + }); + + it('keeps a label name that repeats across kinds independent per store', async () => { + await applyIntent(intent(), context(IMAGE_ID)); + + const ruler = onlyTool(useRulerStore()); + const rectangle = onlyTool(useRectangleStore()); + expect(ruler.labelName).toBe('roi'); + expect(rectangle.labelName).toBe('roi'); + expect(ruler.label).not.toBe(rectangle.label); + + // addTool re-reads the style from the merged label, so these ARE the + // namespaced styles that landed. + expect(ruler.color).toBe('#ff0000'); + expect(ruler.strokeWidth).toBe(3); + expect(rectangle.color).toBe('#00ff00'); + expect(rectangle.fillColor).toBe('#00ff0033'); + expect(onlyTool(usePolygonStore()).color).toBe('#0000ff'); + }); + + it('merges into an existing label of the same name instead of duplicating it', async () => { + const rulerStore = useRulerStore(); + // 'Label 1' ships as the stores' default label. + const [existingId] = Object.keys(rulerStore.labels); + const before = Object.keys(rulerStore.labels).length; + + const file = annotationsFile(); + file.labels.rulers = { 'Label 1': { color: '#123456', strokeWidth: 3 } }; + file.tools.rulers[0].labelName = 'Label 1'; + file.tools.rectangles = []; + file.tools.polygons = []; + serveFile(file); + + expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( + 'applied' + ); + expect(Object.keys(rulerStore.labels)).toHaveLength(before); + expect(rulerStore.labels[existingId].color).toBe('#123456'); + expect(onlyTool(rulerStore).label).toBe(existingId); + }); + + it('leaves the label picker where the user left it', async () => { + const rulerStore = useRulerStore(); + const activeBefore = rulerStore.activeLabel; + expect(activeBefore).toBeTruthy(); + + const file = annotationsFile(); + // A name no store label carries, so merging must ADD one — the case that + // could steal the active selection. + file.labels.rulers = { fresh: { color: '#abcdef' } }; + file.tools.rulers[0].labelName = 'fresh'; + serveFile(file); + + expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( + 'applied' + ); + expect(rulerStore.activeLabel).toBe(activeBefore); + // The label still landed; only the picker was left alone. + expect(onlyTool(rulerStore).labelName).toBe('fresh'); + }); + + it('leaves an unlabeled tool unlabeled', async () => { + const file = annotationsFile(); + file.labels = { rulers: {}, rectangles: {}, polygons: {} }; + file.tools.rulers = [ + { + firstPoint: [1, 1, 5], + secondPoint: [4, 4, 5], + frameOfReference: axialAt(5), + }, + ]; + file.tools.rectangles = []; + file.tools.polygons = []; + serveFile(file); + + expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( + 'applied' + ); + const ruler = onlyTool(useRulerStore()); + expect(ruler.label).toBe(''); + expect(ruler.labelName).toBe(''); + }); + + it('is a no-op when a tool already carries the same source', async () => { + expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( + 'applied' + ); + mocks.fetchProcessingResult.mockClear(); + + const second = await applyIntent(intent(), context(IMAGE_ID)); + expect(second.status).toBe('applied'); + expect(toolCounts()).toEqual({ rulers: 1, rectangles: 1, polygons: 1 }); + // The receipt short-circuits before the download. + expect(mocks.fetchProcessingResult).not.toHaveBeenCalled(); + }); + + it('re-applies a result from a different job even at the same output id', async () => { + await applyIntent(intent(), context(IMAGE_ID)); + const other = { ...source, jobId: 'job-2' }; + await applyIntent(intent({ source: other }), context(IMAGE_ID)); + expect(toolCounts()).toEqual({ rulers: 2, rectangles: 2, polygons: 2 }); + }); + + it('applies an empty result as a no-op', async () => { + serveFile({ schemaVersion: 1, space: 'LPS', tools: {} }); + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('applied'); + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + }); + + it('fails without ever downloading when no image is bound', async () => { + const outcome = await applyIntent(intent(), context(undefined)); + expect(outcome.status).toBe('failed'); + expect(String((outcome as { error: Error }).error)).toContain( + "Load the job's input image" + ); + expect(mocks.fetchProcessingResult).not.toHaveBeenCalled(); + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + }); + + it('fails when the bound image is no longer in the cache', async () => { + const outcome = await applyIntent(intent(), context('img-gone')); + expect(outcome.status).toBe('failed'); + expect(mocks.fetchProcessingResult).not.toHaveBeenCalled(); + }); + + it('fails on a malformed result body without touching the stores', async () => { + serveFile('not json at all'); + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('failed'); + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + }); + + it('rejects the whole result when any frame is not axis-aligned, before mutating', async () => { + const rulerStore = useRulerStore(); + const labelsBefore = { ...rulerStore.labels }; + + const file = annotationsFile(); + // Oblique: unrenderable, and no `slice` echo can rescue it. + file.tools.polygons[0].frameOfReference = { + planeNormal: [0, 0.7071, 0.7071], + planeOrigin: [0, 0, 3], + }; + serveFile(file); + + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('failed'); + expect(String((outcome as { error: Error }).error)).toContain( + 'not aligned' + ); + // All-or-nothing: not even the rulers that WOULD have placed, and not the + // labels — merging restyles, so it is a mutation too. + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + expect(rulerStore.labels).toEqual(labelsBefore); + }); + + it('places a plane past the image bounds, as the renderer already does', async () => { + const file = annotationsFile(); + file.tools.rectangles = []; + file.tools.polygons = []; + file.tools.rulers[0].frameOfReference = axialAt(500); + serveFile(file); + + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + + expect(outcome.status).toBe('applied'); + expect(onlyTool(useRulerStore()).slice).toBe(500); + }); + + it('rejects a plane that falls between slices, and says so', async () => { + const file = annotationsFile(); + file.tools.rulers[0].frameOfReference = axialAt(5.5); + serveFile(file); + + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + + expect(outcome.status).toBe('failed'); + expect(String((outcome as { error: Error }).error)).toContain( + 'between slices' + ); + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + }); + + it('rejects a dangling label reference', async () => { + const file = annotationsFile(); + file.tools.rulers[0].labelName = 'undeclared'; + serveFile(file); + expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( + 'failed' + ); + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + }); + + it('refuses session-only state on the wire, so it can never reach a store', async () => { + const file = annotationsFile(); + Object.assign(file.tools.rulers[0], { + id: 'smuggled', + imageID: 'some-other-image', + color: '#000000', + hidden: true, + source: 'x:y:z', + }); + serveFile(file); + + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('failed'); + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + }); + + it('applies without a source when the producer omitted one', async () => { + const outcome = await applyIntent( + intent({ source: undefined }), + context(IMAGE_ID) + ); + expect(outcome.status).toBe('applied'); + expect(onlyTool(useRulerStore()).source).toBeUndefined(); + }); + + // A stored `frame` flips a tool into cine semantics (render slice, + // visibility, jump-to), so the preflight judges it against the TARGET image. + describe('the advisory frame against the target image', () => { + const markCine = (frames: number, id = IMAGE_ID) => { + useDICOMStore().volumeInfo[id] = { + NumberOfSlices: frames, + VolumeID: id, + Modality: 'US', + SeriesInstanceUID: '1.2.3.4', + SeriesNumber: '1', + SeriesDescription: 'clip', + WindowLevel: '128', + WindowWidth: '256', + kind: 'cine', + }; + }; + + const rulerOnlyFile = (frame?: unknown) => { + const file = annotationsFile(); + file.tools.rulers = [ + { + firstPoint: [1, 1, 5], + secondPoint: [4, 4, 5], + frameOfReference: axialAt(5), + ...(frame === undefined ? {} : { frame }), + }, + ]; + file.labels.rulers = {}; + file.tools.rectangles = []; + file.tools.polygons = []; + serveFile(file); + }; + + it('drops a stray frame when the target is a static volume', async () => { + rulerOnlyFile(3); + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('applied'); + expect(onlyTool(useRulerStore()).frame).toBeUndefined(); + }); + + it('keeps an in-range integral frame on a cine target', async () => { + markCine(8); + rulerOnlyFile(7); + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('applied'); + expect(onlyTool(useRulerStore()).frame).toBe(7); + }); + + it('applies a frameless tool to a cine target (every frame)', async () => { + markCine(8); + rulerOnlyFile(); + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('applied'); + expect(onlyTool(useRulerStore()).frame).toBeUndefined(); + }); + + // Fractional and negative frames are not frames at all, so they die in the + // wire decoder and take the whole result with them. + it.each([ + ['fractional', 1.5], + ['negative', -1], + ])( + 'rejects the whole result for a %s frame on a cine target', + async (_label, frame) => { + markCine(8); + rulerOnlyFile(frame); + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('failed'); + // All-or-nothing: nothing may land. + expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); + } + ); + + // A frame beyond the clip is only judgeable against the target image, and + // the contract makes it advisory: drop it rather than lose the result. + it('drops an out-of-range frame on a cine target', async () => { + markCine(8); + rulerOnlyFile(8); + const outcome = await applyIntent(intent(), context(IMAGE_ID)); + expect(outcome.status).toBe('applied'); + expect(onlyTool(useRulerStore()).frame).toBeUndefined(); + }); + }); +}); diff --git a/src/processing/__tests__/applyResults.spec.ts b/src/processing/__tests__/applyResults.spec.ts index 634e8300b..f77c0fb4d 100644 --- a/src/processing/__tests__/applyResults.spec.ts +++ b/src/processing/__tests__/applyResults.spec.ts @@ -8,6 +8,7 @@ import type { ProcessingResult, SubmittedJobContext, } from '@/src/processing/types'; +import type { ProcessingResultSource } from '@/src/types'; const mocks = vi.hoisted(() => ({ uriToDataSource: vi.fn(), @@ -19,10 +20,7 @@ const mocks = vi.hoisted(() => ({ removeDataset: vi.fn(), convertImageToLabelmap: vi.fn(), updateSegment: vi.fn(), - metadataByID: {} as Record< - string, - { source?: { providerId: string; jobId: string; outputId: string } } - >, + metadataByID: {} as Record, addError: vi.fn(), })); @@ -206,7 +204,7 @@ describe('applyIntent', () => { expect(mocks.updateSegment).not.toHaveBeenCalled(); }); - it('stamps the provider-qualified source tag on the created group', async () => { + it('stamps structured provider-qualified provenance on the created group', async () => { const source = { providerId: 'p1', jobId: 'job-abc123', @@ -229,7 +227,9 @@ describe('applyIntent', () => { jobId: 'job-abc123', outputId: 'outputLabelmap', }; - mocks.metadataByID = { restored: { source } }; + mocks.metadataByID = { + restored: { source }, + }; const outcome = await applyIntent( { intent: 'add-segment-group', ...file, source }, @@ -274,11 +274,7 @@ describe('applyIntent', () => { it('applies matching raw job and output ids from a different provider', async () => { mocks.metadataByID = { restored: { - source: { - providerId: 'provider-a', - jobId: '1', - outputId: 'seg', - }, + source: { providerId: 'provider-a', jobId: '1', outputId: 'seg' }, }, }; const source = { diff --git a/src/processing/applyResults.ts b/src/processing/applyResults.ts index 3afd66049..1b7f48c8e 100644 --- a/src/processing/applyResults.ts +++ b/src/processing/applyResults.ts @@ -1,22 +1,38 @@ import { + ANNOTATION_TOOL_KINDS, + type AnnotationLabel, + type AnnotationToolKind, type KnownResultIntent, + type ResultSource, type SegmentDescriptor, + type WirePolygon, + type WireRuler, } from '@/backend-contract'; import type { ProcessingResult, SubmittedJobContext, } from '@/src/processing/types'; import { resultToIntent } from '@/src/processing/engine/resultToIntent'; -import { ensureError } from '@/src/utils'; +import { + decodeAnnotationsFile, + type DecodedAnnotationsFile, +} from '@/src/processing/engine/annotationsWire'; +import { fetchProcessingResult } from '@/src/processing/engine/resultDownload'; +import { annotationToolStore } from '@/src/processing/annotationKinds'; +import { cleanUndefined, ensureError } from '@/src/utils'; +import { frameOfReferenceToImageSliceAndAxis } from '@/src/utils/frameOfReference'; import { uriToDataSource } from '@/src/io/import/dataSource'; import { importVolumeDataSources, toDataSelection, } from '@/src/io/import/importDataSources'; import { isVolumeResult } from '@/src/io/import/common'; +import type { ImageMetadata } from '@/src/types/image'; import { useDatasetStore } from '@/src/store/datasets'; +import { useDICOMStore } from '@/src/store/datasets-dicom'; import { useLayersStore } from '@/src/store/datasets-layers'; import { useSegmentGroupStore } from '@/src/store/segmentGroups'; +import { useImageCacheStore } from '@/src/store/image-cache'; import { useMessageStore } from '@/src/store/messages'; import { loadVolumeUrls } from '@/src/actions/loadUserFiles'; @@ -26,18 +42,27 @@ type SegmentGroupIntent = Extract< KnownResultIntent, { intent: 'add-segment-group' } >; +type AnnotationsIntent = Extract< + KnownResultIntent, + { intent: 'add-annotations' } +>; export type ApplyIntentOutcome = | { status: 'applied' } | { status: 'failed'; error: unknown }; +const sameResultSource = ( + source: ResultSource | undefined, + target: ResultSource +): boolean => + source?.providerId === target.providerId && + source.jobId === target.jobId && + source.outputId === target.outputId; + function segmentGroupResultInScene(intent: SegmentGroupIntent): boolean { const target = intent.source; if (!target) return false; - return Object.values(useSegmentGroupStore().metadataByID).some( - ({ source }) => - source?.providerId === target.providerId && - source.jobId === target.jobId && - source.outputId === target.outputId + return Object.values(useSegmentGroupStore().metadataByID).some(({ source }) => + sameResultSource(source, target) ); } @@ -88,6 +113,247 @@ async function convertAndDescribe( return ids; } +// Annotation results are fully decoded and located before labels or tools are +// mutated. Store payloads remain explicit allowlists of decoded fields. + +// Session-restored tools retain their result source, so that durable +// provenance doubles as an application receipt: re-Loading a job adds nothing. +function annotationResultInScene(intent: AnnotationsIntent): boolean { + const target = intent.source; + if (!target) return false; + return ANNOTATION_TOOL_KINDS.some((kind) => + Object.values(annotationToolStore(kind).toolByID).some(({ source }) => + sameResultSource(source, target) + ) + ); +} + +type PreparedCore = { + imageID: string; + slice: number; + frameOfReference: WireRuler['frameOfReference']; + labelName?: string; + frame?: number; + name?: string; + metadata?: Record; +}; + +// Geometry travels as one opaque bag so the code below never re-branches on +// kind: only the store the bag is handed to knows its shape, and the kind table +// is what pairs the two. +type PreparedGeometry = + | Pick + | Pick; + +type PreparedTool = PreparedCore & { geometry: PreparedGeometry }; + +type PreparedAnnotations = Record; + +// Frame count of a cine target, or null for a static volume. Reads the same +// record `isCineImage` keys on; for 'cine', NumberOfSlices is the frame count. +const cineFrameCountFor = (imageID: string): number | null => { + const info = useDICOMStore().volumeInfo[imageID]; + return info?.kind === 'cine' ? info.NumberOfSlices : null; +}; + +/** + * A stored `frame` flips a tool into cine semantics everywhere (render slice, + * visibility, jump-to drives playback), so its validity depends on the TARGET + * image, not the producer. It stays an advisory echo the client never trusts: + * on a static volume it is dropped, and on a cine image a frame the clip does + * not have is dropped too, leaving the tool on every frame — the same place an + * absent frame puts it. + */ +const prepareFrame = ( + frame: number | undefined, + cineFrameCount: number | null +): number | undefined => { + if (cineFrameCount == null || frame == null) return undefined; + const inClip = + Number.isInteger(frame) && frame >= 0 && frame < cineFrameCount; + return inClip ? frame : undefined; +}; + +/** + * Locate a wire frame of reference on THIS image, or say why it cannot be. + * Out-of-bounds slices are accepted, matching what the renderer already places; + * an oblique plane and a plane between slices are both unrenderable, and they + * are distinguished so the failure names its own cause. + */ +const locateAnnotationPlane = ( + frameOfReference: WireRuler['frameOfReference'], + imageMetadata: ImageMetadata +): { slice: number } => { + const located = frameOfReferenceToImageSliceAndAxis( + frameOfReference, + imageMetadata, + { allowOutOfBoundsSlice: true } + ); + if (located) return located; + // Only the non-integral slice is forgiven by the second probe, so an answer + // here means the plane was axis-aligned all along. + const alignedButBetweenSlices = frameOfReferenceToImageSliceAndAxis( + frameOfReference, + imageMetadata, + { allowOutOfBoundsSlice: true, allowNonIntegralSlice: true } + ); + throw new Error( + alignedButBetweenSlices + ? 'Annotation plane falls between slices of the input image' + : 'Annotation plane is not aligned to an axis of the input image' + ); +}; + +/** + * Project one decoded wire tool onto the store's core fields, rejecting the + * whole result if it cannot be placed. `wire.slice` is advisory: the slice is + * re-derived from the frame of reference against THIS image, and a plane no + * slice of that image lies on cannot be rendered — no `slice` fallback can + * make it otherwise. + */ +const prepareCore = ( + tool: WireRuler | WirePolygon, + imageID: string, + imageMetadata: ImageMetadata, + cineFrameCount: number | null +): PreparedCore => { + const located = locateAnnotationPlane(tool.frameOfReference, imageMetadata); + return { + imageID, + slice: located.slice, + frameOfReference: tool.frameOfReference, + ...cleanUndefined({ + labelName: tool.labelName || undefined, + frame: prepareFrame(tool.frame, cineFrameCount), + name: tool.name, + metadata: tool.metadata, + }), + }; +}; + +const wireGeometry = (tool: WireRuler | WirePolygon): PreparedGeometry => + 'points' in tool + ? { points: tool.points } + : { firstPoint: tool.firstPoint, secondPoint: tool.secondPoint }; + +const prepareAnnotations = ( + decoded: DecodedAnnotationsFile, + imageID: string, + imageMetadata: ImageMetadata, + cineFrameCount: number | null +): PreparedAnnotations => + Object.fromEntries( + ANNOTATION_TOOL_KINDS.map((kind) => [ + kind, + decoded.tools[kind].map((tool) => ({ + geometry: wireGeometry(tool), + ...prepareCore(tool, imageID, imageMetadata, cineFrameCount), + })), + ]) + ) as PreparedAnnotations; + +// Label identity across the boundary is the NAME, inside its own tool-kind +// namespace: merging returns the store id a tool must point at. Only names the +// tools actually reference are merged — a declaration nothing uses would be +// clutter in the label picker. +const mergeReferencedLabels = ( + kind: AnnotationToolKind, + tools: readonly PreparedCore[], + namespace: Record +): Record => { + const store = annotationToolStore(kind); + const names = new Set( + tools.flatMap((tool) => (tool.labelName ? [tool.labelName] : [])) + ); + // A merge that lands on a new name adds a label, and adding one activates it. + // Applying a result is not the user picking a label, so the picker is put back. + const activeBefore = store.activeLabel; + const ids = Object.fromEntries( + [...names].map((labelName) => [ + labelName, + store.mergeLabel({ labelName, ...(namespace[labelName] ?? {}) }), + ]) + ); + store.setActiveLabel(activeBefore); + return ids; +}; + +// `labelName` is deliberately NOT passed through: addTool re-derives it from +// the label id, and passing a name without an id would silently blank it. +const toolPayload = ( + { labelName, ...core }: PreparedCore, + labelIds: Record, + source: ResultSource | undefined +) => ({ + ...core, + label: (labelName && labelIds[labelName]) || '', + ...(source ? { source } : {}), +}); + +async function applyAnnotations( + intent: AnnotationsIntent, + parentSelection: string | undefined +): Promise { + if (annotationResultInScene(intent)) return { status: 'applied' }; + + // Tools are anchored to an image; without one they would be orphans the UI + // never shows. Opening the file as a dataset is not a fallback either — it is + // not an image. + const imageMetadata = parentSelection + ? useImageCacheStore().getImageMetadata(parentSelection) + : null; + if (!parentSelection || !imageMetadata) { + return { + status: 'failed', + error: new Error( + "Load the job's input image before applying annotations" + ), + }; + } + + const file = await fetchProcessingResult({ + id: intent.id, + name: intent.name, + url: intent.url, + }); + const decoded = decodeAnnotationsFile(JSON.parse(await file.text())); + + const prepared = prepareAnnotations( + decoded, + parentSelection, + imageMetadata, + cineFrameCountFor(parentSelection) + ); + // A task that found nothing to annotate succeeded; there is just no state to add. + if (ANNOTATION_TOOL_KINDS.every((kind) => prepared[kind].length === 0)) { + return { status: 'applied' }; + } + + // Labels first for every kind, then the tools: a tool points at the store id + // its label merged to. + const labelIds = Object.fromEntries( + ANNOTATION_TOOL_KINDS.map((kind) => [ + kind, + mergeReferencedLabels(kind, prepared[kind], decoded.labels[kind]), + ]) + ) as Record>; + + ANNOTATION_TOOL_KINDS.forEach((kind) => { + const store = annotationToolStore(kind); + prepared[kind].forEach(({ geometry, ...core }) => { + // Held in a local so the geometry reaches the store: the registry's + // uniform tool type does not carry the per-kind geometry keys. + const payload = { + ...geometry, + ...toolPayload(core, labelIds[kind], intent.source), + }; + store.addTool(payload); + }); + }); + + return { status: 'applied' }; +} + export async function applyIntent( intent: KnownResultIntent, context: SubmittedJobContext | undefined @@ -150,6 +416,9 @@ export async function applyIntent( useDatasetStore().remove(childSelection); } } + case 'add-annotations': { + return await applyAnnotations(intent, parentSelection); + } default: { const exhaustive: never = intent; void exhaustive; diff --git a/src/store/__tests__/annotationToolImageDelete.spec.ts b/src/store/__tests__/annotationToolImageDelete.spec.ts index 2af22d133..b7608a1e4 100644 --- a/src/store/__tests__/annotationToolImageDelete.spec.ts +++ b/src/store/__tests__/annotationToolImageDelete.spec.ts @@ -42,6 +42,7 @@ const makeRuler = ( | 'hidden' | 'metadata' | 'frame' + | 'source' > => ({ firstPoint: [1, 1, 1], secondPoint: [2, 2, 2], diff --git a/src/store/__tests__/rulers.spec.ts b/src/store/__tests__/rulers.spec.ts index fe4001271..f47d88284 100644 --- a/src/store/__tests__/rulers.spec.ts +++ b/src/store/__tests__/rulers.spec.ts @@ -16,6 +16,7 @@ function createRuler(): RequiredWithPartial< | 'hidden' | 'metadata' | 'frame' + | 'source' > { return { firstPoint: [1, 1, 1], diff --git a/src/store/segmentGroups.ts b/src/store/segmentGroups.ts index f37eee0a3..98905ef69 100644 --- a/src/store/segmentGroups.ts +++ b/src/store/segmentGroups.ts @@ -9,6 +9,7 @@ import { useIdStore } from '@/src/store/id'; import { onImageDeleted } from '@/src/composables/onImageDeleted'; import { normalizeForStore, removeFromArray } from '@/src/utils'; import { SegmentMask } from '@/src/types/segment'; +import type { ProcessingResultSource } from '@/src/types'; import { DEFAULT_SEGMENT_MASKS, CATEGORICAL_COLORS } from '@/src/config'; import { readImage, writeSegmentation } from '@/src/io/readWriteImage'; import { @@ -54,16 +55,8 @@ export type SegmentGroupMetadata = { order: number[]; byValue: Record; }; - // Provenance of a job-produced segment group. This is the durable - // idempotency key used to avoid reapplying a restored job output. Optional + - // additive — hand-painted groups have none. Flows through addLabelmap and - // round-trips the `.volview.zip` (see the matching `SegmentGroupSource` in - // io/state-file/schema.ts). - source?: { - providerId: string; - jobId: string; - outputId: string; - }; + // Provenance of a job-produced group; absent on hand-painted ones. + source?: ProcessingResultSource; }; export function createLabelmapFromImage(imageData: vtkImageData) { diff --git a/src/store/tools/useLabels.ts b/src/store/tools/useLabels.ts index 544653e17..4731d4074 100644 --- a/src/store/tools/useLabels.ts +++ b/src/store/tools/useLabels.ts @@ -22,7 +22,9 @@ export const useLabels = (newLabelDefault: Props) => { const labels = ref({}); const activeLabel = ref(); - const setActiveLabel = (id: string) => { + // Accepts undefined so a caller that must not disturb the picker — applying a + // job's annotations result — can put back an activeLabel that was never set. + const setActiveLabel = (id: string | undefined) => { activeLabel.value = id; }; @@ -115,6 +117,9 @@ export const useLabels = (newLabelDefault: Props) => { addLabel, deleteLabel, updateLabel, + // Exposed for callers that need the merged label's id back — applying a + // job's annotations result maps wire label NAMES to store label ids. + mergeLabel, mergeLabels, findLabel, clearDefaultLabels, diff --git a/src/types/annotation-tool.ts b/src/types/annotation-tool.ts index dc2095018..e487965bd 100644 --- a/src/types/annotation-tool.ts +++ b/src/types/annotation-tool.ts @@ -1,4 +1,5 @@ import { FrameOfReference } from '../utils/frameOfReference'; +import type { ProcessingResultSource } from './index'; export type ToolID = string & { __type: 'ToolID' }; @@ -36,4 +37,7 @@ export type AnnotationTool = { * Arbitrary key-value pairs associated with the annotation. */ metadata?: Record; + + // Provenance of a job-produced tool; absent on hand-placed ones. + source?: ProcessingResultSource; }; diff --git a/src/types/index.ts b/src/types/index.ts index 24ecb92b2..da5c51b35 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -8,6 +8,20 @@ export type NullableValues = { [K in keyof T]: T[K] | null; }; +/** + * Provenance of session state produced by a processing job: the durable + * idempotency identity that keeps a restored job output from being applied + * twice. Optional wherever it is used; hand-made state has none. + * + * Structurally mirrors the backend contract's ResultSource without making the + * core scene types depend on the processing contract. + */ +export type ProcessingResultSource = { + providerId: string; + jobId: string; + outputId: string; +}; + export type SampleDataset = { name: string; filename: string; From 1715428060044069f3f407500584a2a5d04b2943 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 3 Aug 2026 23:32:55 -0400 Subject: [PATCH 4/4] feat(processing): present automatic inputs as key-value rows Render each automatic source as a labeled key and value with a stable two-column layout. Show absent optional inputs as intentional omissions while preserving errors for required inputs. --- src/processing/components/JobsModule.vue | 2 +- .../components/widgets/FileWidget.vue | 77 ++++++++++++++----- .../widgets/__tests__/FileWidget.spec.ts | 73 ++++++++++++++++++ 3 files changed, 130 insertions(+), 22 deletions(-) create mode 100644 src/processing/components/widgets/__tests__/FileWidget.spec.ts diff --git a/src/processing/components/JobsModule.vue b/src/processing/components/JobsModule.vue index b1c0acadf..fd85cc802 100644 --- a/src/processing/components/JobsModule.vue +++ b/src/processing/components/JobsModule.vue @@ -16,7 +16,7 @@ mdi-console-line - Run a job + Run Jobs
-
- {{ bindingMessage }} +
+ {{ kind.icon }} + + {{ kind.caption }}{{ param.required ? '' : ' (optional)' }} + +
+
+ {{ bindingMessage ?? boundDisplayName }}
-
@@ -34,8 +34,8 @@ const props = defineProps<{ boundType?: BoundSourceRefType; }>(); -// How each bound kind names itself: the icon, the caption under the bound name, -// and the noun the binding sentences use. +// How each bound kind names itself in the key column and the noun the binding +// sentences use. const KINDS: Record< string, { icon: string; caption: string; noun: SourceRefNoun } @@ -47,7 +47,7 @@ const KINDS: Record< }, [TYPE_TAG_ANNOTATIONS]: { icon: 'mdi-ruler', - caption: 'Annotations on the active dataset', + caption: 'Annotations', noun: 'annotation', }, }; @@ -69,26 +69,61 @@ const kind = computed(() => { return (type ? KINDS[type] : undefined) ?? IMAGE_KIND; }); +const OPTIONAL_UNBOUND_STATES = new Set([ + 'unbound', + 'no-segment-group', + 'no-annotations', + 'no-reference-input', +]); + +// Missing optional inputs are an intentional task state, not an instruction +// the user must satisfy. Provenance failures and ambiguous bindings still show +// as errors because those states block even optional source refs. +const optionalUnbound = computed( + () => + !props.param.required && + props.binding != null && + OPTIONAL_UNBOUND_STATES.has(props.binding) +); + const bindingMessage = computed(() => - props.binding + props.binding && !optionalUnbound.value ? bindingStateMessage(props.binding, kind.value.noun) : undefined ); + +const boundDisplayName = computed(() => + optionalUnbound.value + ? 'Not provided' + : (props.boundName ?? kind.value.caption) +); diff --git a/src/processing/components/widgets/__tests__/FileWidget.spec.ts b/src/processing/components/widgets/__tests__/FileWidget.spec.ts new file mode 100644 index 000000000..42904da7f --- /dev/null +++ b/src/processing/components/widgets/__tests__/FileWidget.spec.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from 'vitest'; +import { shallowMount } from '@vue/test-utils'; +import { defineComponent } from 'vue'; + +import FileWidget from '@/src/processing/components/widgets/FileWidget.vue'; +import type { VolViewTaskParameter } from '@/backend-contract'; + +const annotationsParam = (required: boolean): VolViewTaskParameter => ({ + kind: 'sourceRef', + id: 'inputAnnotations', + accepts: ['annotations'], + required, +}); + +const IconStub = defineComponent({ + name: 'VIcon', + template: '', +}); + +const global = { stubs: { VIcon: IconStub } }; + +const mountWidget = (required: boolean, binding = 'no-annotations' as const) => + shallowMount(FileWidget, { + props: { + param: annotationsParam(required), + modelValue: null, + binding, + }, + global, + }); + +describe('FileWidget optional source refs', () => { + it('renders an automatic input as an icon-bearing key and a value', () => { + const wrapper = shallowMount(FileWidget, { + props: { + param: { + kind: 'sourceRef', + id: 'inputVolume', + accepts: ['image'], + required: true, + }, + modelValue: null, + binding: 'bound', + boundName: 'CT Images', + }, + global, + }); + + expect(wrapper.get('.key-text').text()).toBe('Active dataset'); + expect(wrapper.get('.input-key').text()).toContain('mdi-image-outline'); + expect(wrapper.get('.input-value').text()).not.toContain( + 'mdi-image-outline' + ); + expect(wrapper.get('.input-value').text()).toContain('CT Images'); + }); + + it('renders an absent optional annotations input as an intentional omission', () => { + const wrapper = mountWidget(false); + + expect(wrapper.get('.key-text').text()).toBe('Annotations (optional)'); + expect(wrapper.get('.input-value').text()).toBe('Not provided'); + expect(wrapper.text()).not.toMatch(/place a ruler/i); + expect(wrapper.get('.input-value').classes()).not.toContain('text-error'); + }); + + it('keeps the placement instruction for a required annotations input', () => { + const wrapper = mountWidget(true); + + expect(wrapper.get('.key-text').text()).toBe('Annotations'); + expect(wrapper.get('.input-value').classes()).toContain('text-error'); + expect(wrapper.get('.input-value').text()).toMatch(/place a ruler/i); + }); +});