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/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/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/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 ba37240c7..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": [ { @@ -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": [ @@ -1265,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": {} } ] }, @@ -1558,6 +1672,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/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/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__/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 2bb3d87d4..603c1bed6 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({ @@ -153,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'); }); @@ -168,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'] @@ -216,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/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..413ef453a 100644 --- a/backend-contract/processing/openapi.ts +++ b/backend-contract/processing/openapi.ts @@ -43,6 +43,11 @@ 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 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', }; // The generated schemas carry a per-schema `$schema` dialect marker; OpenAPI 3.1 @@ -327,13 +332,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..27c2d41ba 100644 --- a/backend-contract/processing/wire.ts +++ b/backend-contract/processing/wire.ts @@ -11,12 +11,17 @@ // --------------------------------------------------------------------------- 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 -// 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 @@ -36,21 +41,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 // --------------------------------------------------------------------------- @@ -122,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(), @@ -188,7 +225,19 @@ const addSegmentGroup = z }) .passthrough(); -// The STRICT half of the vocabulary: exactly the v1 state directives, each with its +// `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 // `segments`) carries no state directive rather than being applied as valid. @@ -196,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/__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/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/processing/components/JobsModule.vue b/src/processing/components/JobsModule.vue index 520bfd747..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 ) { // 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..64914c7fd 100644 --- a/src/processing/components/widgets/FileWidget.vue +++ b/src/processing/components/widgets/FileWidget.vue @@ -1,31 +1,28 @@ 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); + }); +}); 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/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; 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. *