diff --git a/docs/api.md b/docs/api.md index 9c85d564..68ffd2db 100644 --- a/docs/api.md +++ b/docs/api.md @@ -493,9 +493,9 @@ One status covers more than one situation, which is the whole reason to branch o **409 - the request is well-formed; the resource's state refuses it.** The remedy is to change that state and resubmit the identical request: finish the outstanding jobs, approve the batch, -promote something into the dataset, pass `confirm=true`. Name and tag collisions are here, as is -`UNSERIALIZABLE_MANIFEST` - the request body is fine and the defect is in state stored long -before, so the remedy is to fix the annotation and publish again. +promote something into the dataset, pass `confirm=true`. Name and tag collisions are here, as are +`UNSERIALIZABLE_MANIFEST` and `RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA` - the request body is fine +and the defect is in stored content, so reconcile the annotations and publish again. **422 - the payload itself is wrong.** A blank name, a schema that declares two classes with one name, an annotation that names a class the batch's pinned version does not have. Media failures @@ -581,7 +581,7 @@ argument for branching on `code`. | **401** | `UNAUTHORIZED` — with a `WWW-Authenticate: Bearer` challenge | | **404** | `PROJECT_NOT_FOUND` · `SCHEMA_NOT_FOUND` · `SCHEMA_DRAFT_NOT_FOUND` · `BATCH_NOT_FOUND` · `JOB_NOT_FOUND` · `INGEST_JOB_NOT_FOUND` · `BACKGROUND_JOB_NOT_FOUND` · `ASSET_NOT_FOUND` · `SOURCE_NOT_FOUND` · `DATASET_NOT_FOUND` · `ANNOTATION_NOT_FOUND` · `RELEASE_NOT_FOUND` · `TOKEN_NOT_FOUND` · `INFERENCE_CONNECTION_NOT_FOUND` · `ASSET_NOT_IN_JOB` · `NO_SPLIT_RECIPE` · `EXPORT_FORMAT_NOT_FOUND` · `THUMBNAIL_NOT_CACHED` · `NOT_FOUND` (no such route) | | **405** | `METHOD_NOT_ALLOWED` | -| **409** | `PROJECT_NAME_TAKEN` · `RELEASE_TAG_TAKEN` · `TOKEN_NAME_TAKEN` · `INFERENCE_CONNECTION_NAME_TAKEN` · `WORKSPACE_ALREADY_EXISTS` · `WORKSPACE_NOT_EMPTY` · `SCHEMA_VERSION_CONFLICT` · `INVALID_TRANSITION` · `STALE_WRITE` · `BATCH_NOT_EDITABLE` · `BATCH_IMMUTABLE` · `BATCH_NOT_IN_ANNOTATION` · `ASSET_NOT_WRITABLE` · `JOB_FINISHED` · `BATCH_NOT_COMPLETE` · `JOB_NOT_COMPLETE` · `EMPTY_BATCH` · `EMPTY_RELEASE` · `CONFIRMATION_REQUIRED` · `DESTRUCTIVE_SCHEMA_CHANGE` · `SCHEMA_CHANGE_WOULD_ORPHAN` · `SCHEMA_HAS_NO_DETECTABLE_CLASS` · `UNSERIALIZABLE_MANIFEST` · `LOSSY_EXPORT_NOT_CONSENTED` · `EXPORT_SOURCE_UNREADABLE` · `INFERENCE_CONNECTION_NOT_DOWNLOADABLE` · `INFERENCE_CONNECTION_NOT_CHECKABLE` · `WEIGHTS_DAMAGED` · `INFERENCE_CONNECTION_NOT_SET_UP` | +| **409** | `PROJECT_NAME_TAKEN` · `RELEASE_TAG_TAKEN` · `TOKEN_NAME_TAKEN` · `INFERENCE_CONNECTION_NAME_TAKEN` · `WORKSPACE_ALREADY_EXISTS` · `WORKSPACE_NOT_EMPTY` · `SCHEMA_VERSION_CONFLICT` · `INVALID_TRANSITION` · `STALE_WRITE` · `BATCH_NOT_EDITABLE` · `BATCH_IMMUTABLE` · `BATCH_NOT_IN_ANNOTATION` · `ASSET_NOT_WRITABLE` · `JOB_FINISHED` · `BATCH_NOT_COMPLETE` · `JOB_NOT_COMPLETE` · `EMPTY_BATCH` · `EMPTY_RELEASE` · `RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA` · `CONFIRMATION_REQUIRED` · `DESTRUCTIVE_SCHEMA_CHANGE` · `SCHEMA_CHANGE_WOULD_ORPHAN` · `SCHEMA_HAS_NO_DETECTABLE_CLASS` · `UNSERIALIZABLE_MANIFEST` · `LOSSY_EXPORT_NOT_CONSENTED` · `EXPORT_SOURCE_UNREADABLE` · `INFERENCE_CONNECTION_NOT_DOWNLOADABLE` · `INFERENCE_CONNECTION_NOT_CHECKABLE` · `WEIGHTS_DAMAGED` · `INFERENCE_CONNECTION_NOT_SET_UP` | | **422** | `VALIDATION_ERROR` · `ASSET_NOT_IN_BATCH` · `ANNOTATION_NOT_FROM_MODEL` · `INVALID_NAME` · `INFERENCE_CONNECTION_INVALID` · `INVALID_SCHEMA` · `UNSUPPORTED_GEOMETRY` · `INVALID_ANNOTATION` · `LABEL_CLASS_NOT_IN_SCHEMA` · `DISALLOWED_GEOMETRY` · `ANNOTATION_GEOMETRY_OUT_OF_BOUNDS` · `DUPLICATE_CLASSIFICATION_TAG` · `MISSING_REQUIRED_ATTRIBUTE` · `UNKNOWN_ATTRIBUTE` · `INVALID_ATTRIBUTE_VALUE` · `INVALID_PARTITION` · `UNKNOWN_JOB_TYPE` · `MEDIA_ERROR` · `UNSUPPORTED_MEDIA` · `CORRUPT_MEDIA` · `UNSUPPORTED_PROMPT` · `PROMPT_POINT_OUT_OF_BOUNDS` | | **503** | `WORKSPACE_BUSY` | | **500** | `WORKSPACE_CORRUPT` · `NOT_A_WORKSPACE` · `WORKSPACE_FORMAT_TOO_NEW` · `WORKSPACE_SCHEMA_MISMATCH` · `ENTITY_NOT_FOUND` · `ENTITY_ALREADY_EXISTS` · `CONSTRAINT_VIOLATED` · `MEDIA_TOOL_UNAVAILABLE` · `LOCAL_INFERENCE_UNAVAILABLE` · `INFERENCE_CONNECTION_NOT_RUNNABLE` · `INFERENCE_OUT_OF_MEMORY` · `INTERNAL_ERROR` | diff --git a/docs/batches.md b/docs/batches.md index 68194834..011604f4 100644 --- a/docs/batches.md +++ b/docs/batches.md @@ -68,8 +68,9 @@ batches.get(batch.id).schema_version # still 2 ``` A schema that evolved mid-batch would change the rules under work already in flight, which is -exactly what versioning exists to prevent. Every annotation written into this batch's jobs is -validated against the pinned version, not against whatever is newest. +exactly what versioning exists to prevent. The pin is the validation contract for this batch's +in-progress work: every annotation written into its jobs is validated against the pinned version, +not against whatever is newest. Approving a project that has no schema raises `SchemaNotFound`. Creating version 1 here would be a second door to a schema, and [schemas.md](schemas.md) has only one. @@ -93,8 +94,9 @@ invalidate anything already drawn. So there is nothing on this path for a manual step to protect — and what the manual step cost was that a class published while somebody was annotating stayed invisible to them until they found `repin`. -A **narrowing** version moves nothing, with `allow_destructive` or without it: -that flag says *publish this*, never *and drag every open batch across it*. +A **narrowing** version moves nothing, with `allow_destructive` or without it: it does not move +an open batch's validation contract. That flag says *publish this*, never *and drag every open +batch across it*. Crossing a narrowing is `repin`, one batch at a time, judged against that batch's own labels. diff --git a/docs/releases.md b/docs/releases.md index 4e4a4309..e401b5e0 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -71,9 +71,13 @@ images ingested on another machine produce a different manifest. The project's **active** version - the highest one - with its classes. Each copied annotation still carries the version its own batch pinned, and those can differ: two batches approved -against two versions can both be promoted into one trunk. The mixture is safe rather than sloppy, -because `SchemaChangeWouldOrphan` refuses to remove a class that annotations still depend on. Every -label in a manifest is still described by the classes in that manifest. +against two versions can both be promoted into one trunk. Publishing validates every copied +annotation against the active schema before freezing the manifest. If any do not conform, it +refuses with the affected class counts; reconcile that content before publishing. + +That publication gate makes newly created manifests internally schema-consistent. Exporters still +defend against undeclared classes when reading archived or externally supplied malformed manifests, +because those documents may not have come through this publication path. ## Verification diff --git a/docs/schemas.md b/docs/schemas.md index e5edf8a4..2257f2e4 100644 --- a/docs/schemas.md +++ b/docs/schemas.md @@ -348,7 +348,13 @@ should not be caught by one `except`: `confirm` stands in front of destroying da `ProjectService.delete`), and this stands in front of narrowing a contract, whose usual remedy is "write a wider version", not "say yes harder". -The second gate has no flag at all: +An open batch keeps the version it pinned, so a destructive version can be published before that +batch writes another annotation. Its later annotation is valid under that batch's pin, not under +the newer active schema. If that content is promoted, release publication validates it against the +active schema and refuses the release with affected class counts until the content is reconciled. + +The second gate has no flag at all when the proposed change would orphan annotations the schema +publication path must preserve: ```python # annotations already exist under 'lane' diff --git a/frontend/ui-core/src/generated/api.ts b/frontend/ui-core/src/generated/api.ts index 2085acc6..54e2cff5 100644 --- a/frontend/ui-core/src/generated/api.ts +++ b/frontend/ui-core/src/generated/api.ts @@ -698,6 +698,12 @@ export interface paths { * published, because writing that value as `null` would lose it silently and * writing it as `NaN` would produce a manifest no other tool can read. The * remedy is to correct the annotation and publish again. + * + * The active schema must also describe every annotation the release would + * freeze. Otherwise publishing is 409 + * `RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA`, with per-class blockers in `detail`. + * Reconcile those annotations or restore a compatible active schema, then + * publish again. */ post: operations["publish_release"]; delete?: never; diff --git a/openapi.json b/openapi.json index c6a01d27..463415f1 100644 --- a/openapi.json +++ b/openapi.json @@ -6874,7 +6874,7 @@ ] }, "post": { - "description": "Freeze the trunk as it stands into an immutable, named snapshot.\n\nWhat is frozen is the content: every member asset, every annotation on it as\nit was, and the schema version those labels were judged against. Deleting an\nannotation afterwards cannot reach backwards into a published release.\n\nPublishing twice from an unchanged dataset produces byte-identical manifests\nand therefore the same `manifest_hash`, because nothing time-, machine- or\nidentity-specific goes inside the document. The tag, the timestamp and the\nbuild live on the release row instead.\n\n`split` is stored as a recipe, not materialized. `GET\n/releases/{release_id}/assignment` cuts the folds on demand, deterministically\nand from the frozen asset set. Fractions must sum to 1.0.\n\nTags are unique per dataset and **case-sensitive**, like a git tag: `v1.0` and\n`V1.0` are two releases, and reusing one is 409 `RELEASE_TAG_TAKEN`. A dataset\nwith no assets is 409 `EMPTY_RELEASE`; zero *annotations* is fine, since\nunlabeled images are legitimate training data. A project with no schema is 404\n`SCHEMA_NOT_FOUND`, because there is no version to pin, and an unknown dataset\nis 404 `DATASET_NOT_FOUND`.\n\nOne refusal is about the labels rather than about the request: an annotation\ncarrying a coordinate canonical JSON cannot express \u2014 a NaN or an infinity \u2014\nis 409 `UNSERIALIZABLE_MANIFEST`, and the message names it. Nothing is\npublished, because writing that value as `null` would lose it silently and\nwriting it as `NaN` would produce a manifest no other tool can read. The\nremedy is to correct the annotation and publish again.", + "description": "Freeze the trunk as it stands into an immutable, named snapshot.\n\nWhat is frozen is the content: every member asset, every annotation on it as\nit was, and the schema version those labels were judged against. Deleting an\nannotation afterwards cannot reach backwards into a published release.\n\nPublishing twice from an unchanged dataset produces byte-identical manifests\nand therefore the same `manifest_hash`, because nothing time-, machine- or\nidentity-specific goes inside the document. The tag, the timestamp and the\nbuild live on the release row instead.\n\n`split` is stored as a recipe, not materialized. `GET\n/releases/{release_id}/assignment` cuts the folds on demand, deterministically\nand from the frozen asset set. Fractions must sum to 1.0.\n\nTags are unique per dataset and **case-sensitive**, like a git tag: `v1.0` and\n`V1.0` are two releases, and reusing one is 409 `RELEASE_TAG_TAKEN`. A dataset\nwith no assets is 409 `EMPTY_RELEASE`; zero *annotations* is fine, since\nunlabeled images are legitimate training data. A project with no schema is 404\n`SCHEMA_NOT_FOUND`, because there is no version to pin, and an unknown dataset\nis 404 `DATASET_NOT_FOUND`.\n\nOne refusal is about the labels rather than about the request: an annotation\ncarrying a coordinate canonical JSON cannot express \u2014 a NaN or an infinity \u2014\nis 409 `UNSERIALIZABLE_MANIFEST`, and the message names it. Nothing is\npublished, because writing that value as `null` would lose it silently and\nwriting it as `NaN` would produce a manifest no other tool can read. The\nremedy is to correct the annotation and publish again.\n\nThe active schema must also describe every annotation the release would\nfreeze. Otherwise publishing is 409\n`RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA`, with per-class blockers in `detail`.\nReconcile those annotations or restore a compatible active schema, then\npublish again.", "operationId": "publish_release", "parameters": [ { diff --git a/src/visionset/cli/_errors.py b/src/visionset/cli/_errors.py index f5d7e630..126342d7 100644 --- a/src/visionset/cli/_errors.py +++ b/src/visionset/cli/_errors.py @@ -35,6 +35,7 @@ DestructiveSchemaChange, LossyExportNotConsented, NotAWorkspace, + ReleaseContentWouldViolateSchema, SchemaChangeWouldOrphan, VisionSetError, ) @@ -93,6 +94,9 @@ "There is no flag for this one. Delete or relabel those annotations " "first, or keep the class and change something else." ), + ReleaseContentWouldViolateSchema: ( + "Reconcile the annotations or restore a compatible active schema." + ), } """A remedy a *terminal* can act on, printed under the error's own sentence. diff --git a/src/visionset/formats/classification/__init__.py b/src/visionset/formats/classification/__init__.py index 9334f8bf..2ffbd427 100644 --- a/src/visionset/formats/classification/__init__.py +++ b/src/visionset/formats/classification/__init__.py @@ -142,10 +142,9 @@ def export( image = f"{IMAGES_DIRNAME}/{fold}/{name}" for annotation in asset.annotations: if annotation.label_class not in declared: - # Cannot happen — `SchemaChangeWouldOrphan` refuses to remove - # a class annotations depend on — but a row naming a class - # outside `classes.txt` would be a silent lie in a file read - # as ground truth. + # Publication rejects new inconsistent manifests, but archived or + # externally supplied manifests can still be malformed. A row + # outside `classes.txt` would be a silent lie to its reader. raise ExportSourceUnreadable( f"asset {asset.asset_id} carries class {annotation.label_class!r}, " f"which the release's schema does not declare" diff --git a/src/visionset/formats/coco/__init__.py b/src/visionset/formats/coco/__init__.py index d5e71cd3..031d9c5c 100644 --- a/src/visionset/formats/coco/__init__.py +++ b/src/visionset/formats/coco/__init__.py @@ -244,10 +244,9 @@ def _annotation( ) -> dict[str, Any] | None: """One COCO annotation row, or ``None`` if this label has no place in one.""" category_id = category_of.get(annotation.label_class) - # A label whose class the manifest does not declare cannot happen — - # `SchemaChangeWouldOrphan` refuses to remove a class annotations depend on — - # but a row carrying a made-up category id would be a silent lie in a file a - # trainer reads as ground truth. + # Publication rejects new inconsistent manifests, but archived or externally + # supplied manifests can still be malformed. An invented category id would + # silently mislabel a file a trainer reads as ground truth. if category_id is None: raise ExportSourceUnreadable( f"asset {asset.asset_id} carries class {annotation.label_class!r}, " diff --git a/src/visionset/formats/lanes/__init__.py b/src/visionset/formats/lanes/__init__.py index e8a093c4..e264ed35 100644 --- a/src/visionset/formats/lanes/__init__.py +++ b/src/visionset/formats/lanes/__init__.py @@ -269,6 +269,7 @@ def _walk( the picture into the label document, and the suffix is sniffed from the bytes rather than taken from the asset's uri — so only ``write_image`` knows it. """ + _validate_manifest_classes(manifest) folds = folds_of(release, manifest) walked = [] for asset in manifest.assets: @@ -278,6 +279,19 @@ def _walk( return walked +def _validate_manifest_classes(manifest: Manifest) -> None: + """Refuse a manifest whose annotations name classes its frozen schema omits.""" + declared = {label_class.name for label_class in manifest.classes} + for asset in manifest.assets: + for annotation in asset.annotations: + if annotation.label_class not in declared: + raise refuse( + asset, + f"carries class {annotation.label_class!r}, which the release's " + "schema does not declare", + ) + + def _write_json(into: Path, asset: ManifestAsset, suffix: str, document: object) -> None: into.mkdir(parents=True, exist_ok=True) (into / f"{asset.content_hash}{suffix}").write_text( diff --git a/src/visionset/formats/voc/__init__.py b/src/visionset/formats/voc/__init__.py index ed2d6fdb..f088bcbf 100644 --- a/src/visionset/formats/voc/__init__.py +++ b/src/visionset/formats/voc/__init__.py @@ -250,9 +250,9 @@ def _as_box( ) -> BboxGeometry | None: """The box this annotation contributes, or ``None`` if it contributes none.""" if annotation.label_class not in declared: - # Cannot happen — `SchemaChangeWouldOrphan` refuses to remove a class - # annotations depend on — but an `` naming a class the release - # does not declare would be a silent lie in a file read as ground truth. + # Publication rejects new inconsistent manifests, but malformed, archival, + # or external manifests can still bypass that gate. An undeclared + # `` would be a silent lie in a file read as ground truth. raise ExportSourceUnreadable( f"asset {asset.asset_id} carries class {annotation.label_class!r}, " f"which the release's schema does not declare" diff --git a/src/visionset/formats/yolo/__init__.py b/src/visionset/formats/yolo/__init__.py index fb7539ff..18f52f89 100644 --- a/src/visionset/formats/yolo/__init__.py +++ b/src/visionset/formats/yolo/__init__.py @@ -159,10 +159,9 @@ def _rows(asset: ManifestAsset, index_of: Mapping[str, int]) -> Iterable[str]: width, height = dimensions_of(asset) for annotation in asset.annotations: index = index_of.get(annotation.label_class) - # A label whose class the manifest does not declare cannot happen — - # `SchemaChangeWouldOrphan` refuses to remove a class annotations depend - # on — but writing an index derived from nothing would be a silent lie in - # a file a trainer reads as ground truth. + # Publication rejects new inconsistent manifests, but archived or externally + # supplied manifests can still be malformed. An invented class index would + # silently mislabel a file a trainer reads as ground truth. if index is None: raise ExportSourceUnreadable( f"asset {asset.asset_id} carries class {annotation.label_class!r}, " diff --git a/src/visionset/kernel/__init__.py b/src/visionset/kernel/__init__.py index d299d6ea..1029f6e2 100644 --- a/src/visionset/kernel/__init__.py +++ b/src/visionset/kernel/__init__.py @@ -62,6 +62,7 @@ ProjectNameTaken, ProjectNotFound, PromptPointOutOfBounds, + ReleaseContentWouldViolateSchema, ReleaseNotFound, ReleaseTagTaken, SchemaChangeWouldOrphan, @@ -139,6 +140,7 @@ "ProjectNotFound", "PromptPointOutOfBounds", "ReleaseNotFound", + "ReleaseContentWouldViolateSchema", "ReleaseTagTaken", "SchemaChangeWouldOrphan", "SchemaDraftNotFound", diff --git a/src/visionset/kernel/domain/__init__.py b/src/visionset/kernel/domain/__init__.py index 274870e7..2126db54 100644 --- a/src/visionset/kernel/domain/__init__.py +++ b/src/visionset/kernel/domain/__init__.py @@ -184,6 +184,7 @@ orphanable_shapes, ) from visionset.kernel.domain.schema_draft import DraftAttribute, DraftLabelClass, SchemaDraft +from visionset.kernel.domain.schema_validation import validate_schema_annotation from visionset.kernel.domain.segmentation import ( AssetSegmentation, Mask, @@ -388,6 +389,7 @@ "SchemaDiff", "SchemaDraft", "SchemaProvenance", + "validate_schema_annotation", "SingleJob", "Source", "SourceKind", diff --git a/src/visionset/kernel/domain/release.py b/src/visionset/kernel/domain/release.py index 7c286310..99e5e1ae 100644 --- a/src/visionset/kernel/domain/release.py +++ b/src/visionset/kernel/domain/release.py @@ -142,11 +142,10 @@ class Manifest(BaseModel): ``schema_version`` and ``classes`` are the project's *active* version at publication, while each :class:`ManifestAnnotation` keeps the version its own - batch pinned. Those can differ, because two batches can be approved against - two versions and both promoted into one trunk. The mixture is safe rather - than sloppy: ``SchemaChangeWouldOrphan`` refuses to remove a class that - annotations still depend on, so every label in here is still described by the - classes in here. + batch pinned. Batch writes validate against that pin; release publication + validates every copied annotation against the active schema before creating a + new manifest. Historic manifests remain readable, so exporters also defend + against malformed external or legacy content that did not traverse that gate. **The assets sort themselves**, by content hash and then by id, and each asset's annotations sort by id. That belongs here rather than in the service diff --git a/src/visionset/kernel/domain/schema_validation.py b/src/visionset/kernel/domain/schema_validation.py new file mode 100644 index 00000000..729408cb --- /dev/null +++ b/src/visionset/kernel/domain/schema_validation.py @@ -0,0 +1,64 @@ +"""Validation rules shared by schema-bound annotation writers.""" + +from __future__ import annotations + +from collections.abc import Mapping + +from visionset.kernel.domain.geometry import Geometry +from visionset.kernel.domain.schema import AnnotationSchema, AttributeValue +from visionset.kernel.errors import ( + DisallowedGeometry, + InvalidAttributeValue, + LabelClassNotInSchema, + MissingRequiredAttribute, + UnknownAttribute, +) + + +def validate_schema_annotation( + *, + label_class: str, + geometry: Geometry, + attributes: Mapping[str, AttributeValue], + schema: AnnotationSchema, +) -> None: + """Refuse primitive annotation fields the supplied schema would not recognize. + + Classes and attributes are matched by exact name. Geometry membership is + evaluated against the named class, not the version-wide geometry union. + """ + declared_class = next((item for item in schema.classes if item.name == label_class), None) + if declared_class is None: + known = ", ".join(repr(item.name) for item in schema.classes) or "no classes at all" + raise LabelClassNotInSchema( + f"class {label_class!r} is not in schema version {schema.version}, " + f"which declares {known}" + ) + + if geometry.type not in declared_class.geometries: + allowed = ", ".join(item.value for item in declared_class.geometries) + raise DisallowedGeometry( + f"class {declared_class.name!r} accepts {allowed} in schema version " + f"{schema.version}, but this annotation carries a {geometry.type.value}" + ) + + declared_attributes = {attribute.name: attribute for attribute in declared_class.attributes} + if undeclared := sorted(attributes.keys() - declared_attributes.keys()): + known = ", ".join(repr(name) for name in declared_attributes) or "no attributes at all" + raise UnknownAttribute( + f"class {declared_class.name!r} does not declare " + f"{', '.join(repr(name) for name in undeclared)}; it declares {known}" + ) + + for attribute in declared_class.attributes: + if attribute.name not in attributes: + if attribute.required: + raise MissingRequiredAttribute( + f"class {declared_class.name!r} requires attribute {attribute.name!r}; " + "its default is what a surface should offer, not a value the kernel fills in" + ) + continue + if (reason := attribute.rejects(attributes[attribute.name])) is not None: + raise InvalidAttributeValue( + f"attribute {attribute.name!r} of class {declared_class.name!r} {reason}" + ) diff --git a/src/visionset/kernel/errors.py b/src/visionset/kernel/errors.py index d070811b..ef77e278 100644 --- a/src/visionset/kernel/errors.py +++ b/src/visionset/kernel/errors.py @@ -295,6 +295,17 @@ def __init__(self, message: str, *, blockers: object | None = None) -> None: self.blockers = blockers +class ReleaseContentWouldViolateSchema(VisionSetError): + """A release's active schema does not describe its candidate annotations.""" + + blockers: object | None = None + + def __init__(self, message: str, *, blockers: object | None = None) -> None: + super().__init__(message) + if blockers is not None: + self.blockers = blockers + + class InvalidTransition(VisionSetError): """A state machine was asked to make a move that is not in its table. diff --git a/src/visionset/kernel/services/annotation_service.py b/src/visionset/kernel/services/annotation_service.py index d6bbd4fd..ea772a84 100644 --- a/src/visionset/kernel/services/annotation_service.py +++ b/src/visionset/kernel/services/annotation_service.py @@ -85,19 +85,15 @@ class is not in the version, whose geometry is not the one that class declares, progress_after_annotating, ) from visionset.kernel.domain.geometry import geometry_intersects_asset +from visionset.kernel.domain.schema_validation import validate_schema_annotation from visionset.kernel.errors import ( AnnotationGeometryOutOfBounds, AnnotationNotFound, AnnotationNotFromModel, AssetNotInJob, AssetNotWritable, - DisallowedGeometry, DuplicateClassificationTag, - InvalidAttributeValue, - LabelClassNotInSchema, - MissingRequiredAttribute, StaleWrite, - UnknownAttribute, VisionSetError, WorkspaceCorrupt, ) @@ -586,10 +582,10 @@ def _tags_already_on( replaced is itself one of the stored ones, and judging a replacement against a set that still contains it would refuse every no-op edit. - Reads the store, which is the one thing ``_validate`` deliberately never - does — so it lives here, beside its callers, rather than inside it. That - property is stated in ``_validate``'s own docstring and is worth keeping: - schema judgement is a pure function of the annotation and the version. + Reads the store, which is the one thing ``validate_schema_annotation`` + deliberately never does — so it lives here, beside its callers, rather + than inside it. Schema judgement is a pure function of the annotation and + the version. """ skip = ignoring or set() return { @@ -620,56 +616,12 @@ def _require_untagged(tagged: set[tuple[UUID, str]], annotation: Annotation) -> def _validate(annotation: Annotation, schema: AnnotationSchema) -> None: - """Refuse an annotation the pinned schema version would not recognize. - - Classes and attributes are matched by **exact** name, the same way - ``domain/schema_diff.py`` matches them — which is what makes a rename read as - a remove plus an add there, and what makes ``LabelClass.name`` stored - stripped here. - - The geometry rule is membership in **this class's** set. That is not the same - test as ``SchemaService.allowed_geometries``, which is the union across a - version's classes: it answers "what may this project draw?" and would happily - let a polygon through under a class that only accepts boxes. - - Pure, and given the schema rather than reading one, so the whole rule can be - exercised without a workspace. - """ - label_class = next((c for c in schema.classes if c.name == annotation.label_class), None) - if label_class is None: - known = ", ".join(repr(c.name) for c in schema.classes) or "no classes at all" - raise LabelClassNotInSchema( - f"class {annotation.label_class!r} is not in schema version {schema.version}, " - f"which declares {known}" - ) - - if annotation.geometry.type not in label_class.geometries: - allowed = ", ".join(geometry.value for geometry in label_class.geometries) - raise DisallowedGeometry( - f"class {label_class.name!r} accepts {allowed} in schema version " - f"{schema.version}, but this annotation carries a {annotation.geometry.type.value}" - ) - - declared = {attribute.name: attribute for attribute in label_class.attributes} - if undeclared := sorted(annotation.attributes.keys() - declared.keys()): - known = ", ".join(repr(name) for name in declared) or "no attributes at all" - raise UnknownAttribute( - f"class {label_class.name!r} does not declare " - f"{', '.join(repr(name) for name in undeclared)}; it declares {known}" - ) - - for attribute in label_class.attributes: - if attribute.name not in annotation.attributes: - if attribute.required: - raise MissingRequiredAttribute( - f"class {label_class.name!r} requires attribute {attribute.name!r}; " - f"its default is what a surface should offer, not a value the kernel fills in" - ) - continue - if (reason := attribute.rejects(annotation.attributes[attribute.name])) is not None: - raise InvalidAttributeValue( - f"attribute {attribute.name!r} of class {label_class.name!r} {reason}" - ) + validate_schema_annotation( + label_class=annotation.label_class, + geometry=annotation.geometry, + attributes=annotation.attributes, + schema=schema, + ) def _refresh_progress( diff --git a/src/visionset/kernel/services/release_service.py b/src/visionset/kernel/services/release_service.py index c3121f92..562a43a4 100644 --- a/src/visionset/kernel/services/release_service.py +++ b/src/visionset/kernel/services/release_service.py @@ -51,8 +51,10 @@ from visionset import __version__ from visionset.kernel.domain import ( Annotation, + AnnotationSchema, Asset, ClassCompatibility, + ClassCount, ClassExportStatus, Dataset, ExportCompatibility, @@ -70,13 +72,16 @@ canonical_bytes, normalize_name, sha256_hex, + validate_schema_annotation, ) from visionset.kernel.errors import ( ConstraintViolated, EmptyRelease, ExportSourceUnreadable, + InvalidAnnotation, LossyExportNotConsented, NoSplitRecipe, + ReleaseContentWouldViolateSchema, ReleaseNotFound, ReleaseTagTaken, WorkspaceCorrupt, @@ -166,10 +171,19 @@ def publish(self, dataset_id: UUID, tag: str, *, split: SplitRecipe | None = Non f"into it before publishing a release of it" ) + manifest_assets = _manifest_assets(uow, assets) + blockers = _release_schema_blockers(active, manifest_assets) + if blockers: + raise ReleaseContentWouldViolateSchema( + "cannot publish this release: its active schema no longer describes " + "annotations in the dataset", + blockers=blockers, + ) + manifest = Manifest( schema_version=active.version, classes=active.classes, - assets=_manifest_assets(uow, assets), + assets=manifest_assets, ) project_id = dataset.project_id published = uow.releases.add( @@ -614,6 +628,34 @@ def _manifest_assets(uow: UnitOfWork, assets: list[Asset]) -> tuple[ManifestAsse ) +def _release_schema_blockers( + schema: AnnotationSchema, assets: tuple[ManifestAsset, ...] +) -> tuple[ClassCount, ...]: + """Count candidate annotations that the active release schema would reject.""" + annotations: dict[str, int] = {} + affected_assets: dict[str, set[UUID]] = {} + for asset in assets: + for annotation in asset.annotations: + try: + validate_schema_annotation( + label_class=annotation.label_class, + geometry=annotation.geometry, + attributes=annotation.attributes, + schema=schema, + ) + except InvalidAnnotation: + annotations[annotation.label_class] = annotations.get(annotation.label_class, 0) + 1 + affected_assets.setdefault(annotation.label_class, set()).add(asset.asset_id) + return tuple( + ClassCount( + label_class=name, + annotations=annotations[name], + assets=len(affected_assets[name]), + ) + for name in sorted(annotations) + ) + + def _manifest_annotation(annotation: Annotation) -> ManifestAnnotation: """One live label, copied field by field into its frozen twin. @@ -809,11 +851,9 @@ def _compatibility(release: Release, manifest: Manifest, exporter: Exporter) -> for asset in manifest.assets: for annotation in asset.annotations: geometry = GeometryType(annotation.geometry.type) - # A shape the manifest's own `classes` does not declare cannot happen - # — `SchemaChangeWouldOrphan` refuses to remove a class annotations - # depend on, and taking a geometry away from one is destructive for - # the same reason — but a report that dropped a label it could not - # place would be silently wrong, so it is placed by what it carries. + # Publication rejects new inconsistent manifests, but this report also + # reads archived or externally supplied manifests. Dropping an + # undeclared label here would make the report silently wrong. count, assets = per_shape.get((annotation.label_class, geometry), (0, set())) per_shape[(annotation.label_class, geometry)] = ( count + 1, diff --git a/src/visionset/mcp/_errors.py b/src/visionset/mcp/_errors.py index 2ae6630d..7254664c 100644 --- a/src/visionset/mcp/_errors.py +++ b/src/visionset/mcp/_errors.py @@ -55,6 +55,7 @@ DestructiveSchemaChange, LossyExportNotConsented, NotAWorkspace, + ReleaseContentWouldViolateSchema, ThumbnailNotCached, VisionSetError, ) @@ -101,6 +102,10 @@ "which classes would be dropped and how many annotations that is, then " "re-run `export_release` with `allow_lossy=true` to accept the loss." ), + ReleaseContentWouldViolateSchema: ( + "Reconcile the annotations or publish a schema that describes them, then " + "publish the release again." + ), } """A next step this surface can add under the kernel's sentence. diff --git a/src/visionset/server/errors.py b/src/visionset/server/errors.py index 07b0af81..dbe7ef5c 100644 --- a/src/visionset/server/errors.py +++ b/src/visionset/server/errors.py @@ -99,6 +99,7 @@ ProjectNameTaken, ProjectNotFound, PromptPointOutOfBounds, + ReleaseContentWouldViolateSchema, ReleaseNotFound, ReleaseTagTaken, SchemaChangeWouldOrphan, @@ -279,6 +280,7 @@ class ErrorRule: JobNotComplete: ErrorRule(409, "JOB_NOT_COMPLETE"), EmptyBatch: ErrorRule(409, "EMPTY_BATCH"), EmptyRelease: ErrorRule(409, "EMPTY_RELEASE"), + ReleaseContentWouldViolateSchema: ErrorRule(409, "RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA"), ConfirmationRequired: ErrorRule(409, "CONFIRMATION_REQUIRED"), DestructiveSchemaChange: ErrorRule(409, "DESTRUCTIVE_SCHEMA_CHANGE"), SchemaChangeWouldOrphan: ErrorRule(409, "SCHEMA_CHANGE_WOULD_ORPHAN"), @@ -522,6 +524,14 @@ def _detail_for(exc: BaseException) -> dict[str, Any] | None: if isinstance(count, ClassCount) ] } + if isinstance(exc, ReleaseContentWouldViolateSchema) and isinstance(exc.blockers, tuple): + return { + "blockers": [ + ClassCountOut.of(count).model_dump(mode="json") + for count in exc.blockers + if isinstance(count, ClassCount) + ] + } if isinstance(exc, DestructiveSchemaChange) and isinstance(exc.classes, tuple): # Names only. This refusal is raised before anything on disk is consulted # — see the field's own note — so there are no counts to publish, and a diff --git a/src/visionset/server/routes/releases.py b/src/visionset/server/routes/releases.py index dc8aff91..0afb26c5 100644 --- a/src/visionset/server/routes/releases.py +++ b/src/visionset/server/routes/releases.py @@ -113,6 +113,12 @@ def publish_release(workspace: WorkspaceDep, dataset_id: UUID, body: ReleaseCrea published, because writing that value as `null` would lose it silently and writing it as `NaN` would produce a manifest no other tool can read. The remedy is to correct the annotation and publish again. + + The active schema must also describe every annotation the release would + freeze. Otherwise publishing is 409 + `RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA`, with per-class blockers in `detail`. + Reconcile those annotations or restore a compatible active schema, then + publish again. """ release = ReleaseService(workspace).publish( dataset_id, body.tag, split=None if body.split is None else body.split.to_domain() diff --git a/tests/cli/_flow.py b/tests/cli/_flow.py index 0437ac52..3a93871a 100644 --- a/tests/cli/_flow.py +++ b/tests/cli/_flow.py @@ -18,8 +18,11 @@ import json import re +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path +import typer.rich_utils from click.testing import Result from tests.fixtures.media import write_images, write_unsupported_file from typer.testing import CliRunner @@ -47,7 +50,7 @@ NARROW = "40" """The width every invocation renders at, so no test depends on the terminal's.""" -RENDERING = {"COLUMNS": NARROW, "FORCE_COLOR": "1"} +RENDERING = {"TERM": "xterm-256color", "NO_COLOR": None} """The environment every invocation renders in — narrow, and in colour. Three things outside a test's control decide what a rich ``Panel`` looks like, @@ -70,9 +73,24 @@ """ +@contextmanager +def _rich_rendering() -> Iterator[None]: + """Pin Typer's import-time Rich settings for one test invocation.""" + old_force_terminal = typer.rich_utils.FORCE_TERMINAL + old_max_width = typer.rich_utils.MAX_WIDTH + typer.rich_utils.FORCE_TERMINAL = True + typer.rich_utils.MAX_WIDTH = int(NARROW) + try: + yield + finally: + typer.rich_utils.FORCE_TERMINAL = old_force_terminal + typer.rich_utils.MAX_WIDTH = old_max_width + + def run(root: Path, *argv: str) -> Result: """Invoke the real app against a workspace, without asserting anything.""" - return runner.invoke(app, [*argv, "--workspace", str(root)], env=RENDERING) + with _rich_rendering(): + return runner.invoke(app, [*argv, "--workspace", str(root)], env=RENDERING, color=True) def ok(root: Path, *argv: str) -> str: diff --git a/tests/cli/test_release_commands.py b/tests/cli/test_release_commands.py index 9f2c30d8..37eecdda 100644 --- a/tests/cli/test_release_commands.py +++ b/tests/cli/test_release_commands.py @@ -2,6 +2,7 @@ from __future__ import annotations +from io import BytesIO from pathlib import Path import pytest @@ -15,10 +16,16 @@ workspace, ) +from visionset.kernel.domain import Annotation, Asset, BboxGeometry, GeometryType, LabelClass from visionset.kernel.services import ( WORKSPACE_ENV_VAR, + AnnotationService, + BatchService, + DatasetService, + JobService, ProjectService, ReleaseService, + SchemaService, WorkspaceService, ) @@ -44,6 +51,53 @@ def _manifest_blob(root: Path, name: str, tag: str) -> Path: return root / "blobs" / digest[:2] / digest[2:4] / digest +def _schema_incompatible_project(root: Path) -> str: + with WorkspaceService.open(root) as service: + projects = ProjectService(service) + schemas = SchemaService(service) + batches = BatchService(service) + jobs = JobService(service) + annotations = AnnotationService(service) + datasets = DatasetService(service) + project = projects.create("schema-incompatible") + sign = LabelClass(name="sign", geometries=(GeometryType.BBOX,)) + car = LabelClass(name="car", geometries=(GeometryType.BBOX,)) + schemas.create_version(project.id, [sign, car]) + content_hash = service.blob_store.put(BytesIO(b"schema-incompatible")) + with service.unit_of_work() as uow: + asset = uow.assets.add( + Asset( + project_id=project.id, + content_hash=content_hash, + uri="/tmp/schema-incompatible.png", + width=640, + height=480, + ) + ) + batch = batches.create(project.id, "v1 work", [asset.id]) + batches.approve(batch.id) + (job,) = batches.jobs(batch.id) + batches.start(batch.id) + jobs.start(job.id) + schemas.create_version(project.id, [sign], allow_destructive=True) + annotations.add( + job.id, + [ + Annotation( + asset_id=asset.id, + label_class="car", + schema_version=1, + geometry=BboxGeometry(x=1, y=2, width=30, height=40), + provenance="human", + ) + ], + ) + jobs.complete(job.id) + batches.complete(batch.id) + datasets.promote(batch.id) + return project.name + + # --- publish ----------------------------------------------------------------- @@ -70,6 +124,15 @@ def test_publishing_an_empty_trunk_exits_one(root: Path, tmp_path: Path) -> None assert "Error:" in result.stderr +def test_publishing_content_outside_the_active_schema_exits_one_with_a_remedy(root: Path) -> None: + name = _schema_incompatible_project(root) + + result = run(root, "release", "publish", "--tag", "v1.0", "-p", name) + + assert result.exit_code == 1, result.output + assert "Reconcile the annotations or restore a compatible active schema." in result.stderr + + def test_a_repeated_tag_exits_one(root: Path, tmp_path: Path) -> None: # A release is never edited, so the remedy named in the refusal is a new tag. name = published_release(root, tmp_path) diff --git a/tests/formats/test_classification.py b/tests/formats/test_classification.py index 9be2daac..7818e609 100644 --- a/tests/formats/test_classification.py +++ b/tests/formats/test_classification.py @@ -226,6 +226,42 @@ def test_a_class_name_holding_a_newline_is_refused(tmp_path: Path) -> None: fixture.close() +def test_an_undeclared_manifest_class_aborts_before_a_label_row_is_emitted(tmp_path: Path) -> None: + """Archived or externally supplied manifests bypass the publication consistency gate.""" + fixture = Fixture(tmp_path, classes=CLASSES) + fixture.label(DRAWING) + release_id = fixture.publish() + manifest = fixture.releases.manifest(release_id) + first = next(asset for asset in manifest.assets if asset.annotations) + malformed = manifest.model_copy( + update={ + "assets": ( + first.model_copy( + update={ + "annotations": ( + first.annotations[0].model_copy(update={"label_class": "undeclared"}), + ) + } + ), + *manifest.assets[1:], + ) + } + ) + dest = tmp_path / "out" + + with pytest.raises(ExportSourceUnreadable, match="undeclared"): + ClassificationExporter().export( + fixture.releases.get(release_id), + malformed, + dest, + content=fixture.workspace.blob_store.get, + ) + fixture.close() + + assert not (dest / LABELS_FILENAME).exists() + assert not (dest / CLASSES_FILENAME).exists() + + def test_the_plugin_declares_tags_supported_and_reduces_nothing() -> None: """The capability facts the compatibility report is computed against.""" plugin = ClassificationExporter() diff --git a/tests/formats/test_coco.py b/tests/formats/test_coco.py index 5e3f9d43..5b67eff4 100644 --- a/tests/formats/test_coco.py +++ b/tests/formats/test_coco.py @@ -290,6 +290,41 @@ def test_a_missing_blob_aborts_rather_than_writing_a_document_that_is_short( fixture.close() +def test_an_undeclared_manifest_class_aborts_before_a_category_is_emitted(tmp_path: Path) -> None: + """Archived or externally supplied manifests bypass the publication consistency gate.""" + fixture = Fixture(tmp_path) + fixture.label({0: [_box(x=8, y=12, width=16, height=24)]}) + release_id = fixture.publish() + manifest = fixture.releases.manifest(release_id) + first = next(asset for asset in manifest.assets if asset.annotations) + malformed = manifest.model_copy( + update={ + "assets": ( + first.model_copy( + update={ + "annotations": ( + first.annotations[0].model_copy(update={"label_class": "undeclared"}), + ) + } + ), + *manifest.assets[1:], + ) + } + ) + dest = tmp_path / "out" + + with pytest.raises(ExportSourceUnreadable, match="undeclared"): + CocoExporter().export( + fixture.releases.get(release_id), + malformed, + dest, + content=fixture.workspace.blob_store.get, + ) + fixture.close() + + assert not list((dest / ANNOTATIONS_DIRNAME).glob("instances_*.json")) + + # --- splits and the whole tree ------------------------------------------------ diff --git a/tests/formats/test_lanes.py b/tests/formats/test_lanes.py index b9bc92ee..14c87c6b 100644 --- a/tests/formats/test_lanes.py +++ b/tests/formats/test_lanes.py @@ -60,6 +60,8 @@ from visionset.kernel.domain import ( BboxGeometry, ClassificationGeometry, + GeometryType, + LabelClass, Manifest, ManifestAnnotation, ManifestAsset, @@ -143,8 +145,24 @@ def _release() -> Release: ) +def _manifest(asset: ManifestAsset) -> Manifest: + classes: dict[str, set[GeometryType]] = {} + for annotation in asset.annotations: + classes.setdefault(annotation.label_class, set()).add( + GeometryType(annotation.geometry.type) + ) + return Manifest( + schema_version=1, + classes=tuple( + LabelClass(name=name, geometries=tuple(geometries)) + for name, geometries in classes.items() + ), + assets=(asset,), + ) + + def _export(exporter: object, asset: ManifestAsset, dest: Path) -> Path: - manifest = Manifest(schema_version=1, assets=(asset,)) + manifest = _manifest(asset) exporter.export(_release(), manifest, dest, content=_content) # type: ignore[attr-defined] return dest @@ -691,6 +709,28 @@ def test_every_lane_format_refuses_an_asset_with_no_recorded_size_or_ignores_it( _export(exporter(), asset, tmp_path) +@pytest.mark.parametrize("exporter", EXPORTERS, ids=lambda e: str(e.format_name)) +def test_every_lane_format_refuses_an_undeclared_manifest_class_before_writing( + tmp_path: Path, exporter: type +) -> None: + """Removing the shared input gate would silently emit a malformed lane export.""" + asset = _asset(_lane([(1.0, 1.0), (2.0, 40.0)], label_class="undeclared")) + manifest = Manifest( + schema_version=1, + classes=(LabelClass(name="centerline", geometries=(GeometryType.POLYLINE,)),), + assets=(asset,), + ) + dest = tmp_path / "out" + + with pytest.raises( + ExportSourceUnreadable, + match=rf"asset {asset.asset_id} carries class 'undeclared'", + ): + exporter().export(_release(), manifest, dest, content=_content) + + assert not dest.exists() + + def test_the_manifest_asset_ids_never_reach_the_output(tmp_path: Path) -> None: """Filenames are content hashes, the way every other format here names them. diff --git a/tests/formats/test_voc.py b/tests/formats/test_voc.py index 842b028c..c1d93cfe 100644 --- a/tests/formats/test_voc.py +++ b/tests/formats/test_voc.py @@ -332,6 +332,41 @@ def test_a_missing_blob_aborts_rather_than_writing_a_dataset_that_is_short( fixture.close() +def test_an_undeclared_manifest_class_aborts_before_a_voc_object_is_emitted(tmp_path: Path) -> None: + """Archived or externally supplied manifests bypass the publication consistency gate.""" + fixture = Fixture(tmp_path, images=1) + fixture.label({0: [_box(x=8, y=12, width=16, height=24)]}) + release_id = fixture.publish() + manifest = fixture.releases.manifest(release_id) + first = next(asset for asset in manifest.assets if asset.annotations) + malformed = manifest.model_copy( + update={ + "assets": ( + first.model_copy( + update={ + "annotations": ( + first.annotations[0].model_copy(update={"label_class": "undeclared"}), + ) + } + ), + *manifest.assets[1:], + ) + } + ) + dest = tmp_path / "out" + + with pytest.raises(ExportSourceUnreadable, match="undeclared"): + VocExporter().export( + fixture.releases.get(release_id), + malformed, + dest, + content=fixture.workspace.blob_store.get, + ) + fixture.close() + + assert not list((dest / ANNOTATIONS_DIRNAME).glob("*.xml")) + + # --- splits and repeatability ------------------------------------------------- diff --git a/tests/formats/test_yolo.py b/tests/formats/test_yolo.py index f102e9a1..30159243 100644 --- a/tests/formats/test_yolo.py +++ b/tests/formats/test_yolo.py @@ -383,6 +383,44 @@ def test_a_missing_blob_aborts_rather_than_writing_a_dataset_that_is_short( fixture.close() +def test_an_undeclared_manifest_class_aborts_before_a_label_index_is_written( + tmp_path: Path, +) -> None: + """Archived or externally supplied manifests bypass the publication consistency gate.""" + fixture = Fixture(tmp_path) + fixture.label({0: [_box(x=8, y=12, width=16, height=24)]}) + release_id = fixture.publish() + manifest = fixture.releases.manifest(release_id) + first = next(asset for asset in manifest.assets if asset.annotations) + malformed = manifest.model_copy( + update={ + "assets": ( + first.model_copy( + update={ + "annotations": ( + first.annotations[0].model_copy(update={"label_class": "undeclared"}), + ) + } + ), + *manifest.assets[1:], + ) + } + ) + dest = tmp_path / "out" + + with pytest.raises(ExportSourceUnreadable, match="undeclared"): + YoloDetectionExporter().export( + fixture.releases.get(release_id), + malformed, + dest, + content=fixture.workspace.blob_store.get, + ) + fixture.close() + + assert not list((dest / "labels").rglob("*.txt")) + assert not (dest / DATA_FILENAME).exists() + + # --- splits ------------------------------------------------------------------- diff --git a/tests/kernel/test_release_service.py b/tests/kernel/test_release_service.py index c1f6c7d5..02af8012 100644 --- a/tests/kernel/test_release_service.py +++ b/tests/kernel/test_release_service.py @@ -28,6 +28,7 @@ InvalidName, LossyExportNotConsented, NoSplitRecipe, + ReleaseContentWouldViolateSchema, ReleaseNotFound, ReleaseTagTaken, SchemaNotFound, @@ -38,11 +39,14 @@ AnnotationJob, Asset, AssetProgress, + Attribute, BboxGeometry, + ClassCount, ExportCompatibility, GeometryType, LabelClass, Manifest, + ManifestAnnotation, PolygonGeometry, Release, SplitRecipe, @@ -63,6 +67,7 @@ ) SIGN = LabelClass(name="sign", geometries=(GeometryType.BBOX,)) +CAR = LabelClass(name="car", geometries=(GeometryType.BBOX,)) RECIPE = SplitRecipe(train=0.6, val=0.2, test=0.2, seed=42) @@ -137,6 +142,26 @@ def _box(asset_id: UUID) -> Annotation: ) +def _promote_under_superseded_schema( + fixture: Fixture, + old_classes: list[LabelClass], + active_classes: list[LabelClass], + annotation: Annotation, +) -> None: + """Promote one annotation written under v1 after v2 has become active.""" + fixture.schemas.create_version(fixture.project.id, old_classes) + batch = fixture.batches.create(fixture.project.id, "v1 work", [annotation.asset_id]) + fixture.batches.approve(batch.id) + (job,) = fixture.batches.jobs(batch.id) + fixture.batches.start(batch.id) + fixture.jobs.start(job.id) + fixture.schemas.create_version(fixture.project.id, active_classes, allow_destructive=True) + fixture.annotations.add(job.id, [annotation]) + fixture.jobs.complete(job.id) + fixture.batches.complete(batch.id) + fixture.datasets.promote(batch.id) + + # --- what publishing freezes -------------------------------------------------- @@ -169,6 +194,91 @@ def test_publishing_twice_from_an_unchanged_dataset_yields_byte_identical_manife fixture.close() +def test_release_publication_refuses_content_not_described_by_the_active_schema( + tmp_path: Path, +) -> None: + fixture = Fixture(tmp_path) + asset_id = fixture.asset_ids[0] + _promote_under_superseded_schema( + fixture, + [SIGN, CAR], + [SIGN], + Annotation( + asset_id=asset_id, + label_class="car", + schema_version=1, + geometry=BboxGeometry(x=1.0, y=2.0, width=30.0, height=40.0), + provenance="human", + ), + ) + + with pytest.raises(ReleaseContentWouldViolateSchema) as caught: + fixture.releases.publish(fixture.dataset_id, "v1") + + assert caught.value.blockers == (ClassCount(label_class="car", annotations=1, assets=1),) + assert fixture.releases.list(fixture.dataset_id) == [] + fixture.close() + + +def test_release_publication_refuses_a_geometry_removed_from_the_active_class( + tmp_path: Path, +) -> None: + fixture = Fixture(tmp_path) + asset_id = fixture.asset_ids[0] + car_with_polygon = LabelClass(name="car", geometries=(GeometryType.BBOX, GeometryType.POLYGON)) + _promote_under_superseded_schema( + fixture, + [SIGN, car_with_polygon], + [SIGN, CAR], + Annotation( + asset_id=asset_id, + label_class="car", + schema_version=1, + geometry=PolygonGeometry(points=[(0.0, 0.0), (10.0, 0.0), (10.0, 10.0)]), + provenance="human", + ), + ) + + with pytest.raises(ReleaseContentWouldViolateSchema) as caught: + fixture.releases.publish(fixture.dataset_id, "v1") + + assert caught.value.blockers == (ClassCount(label_class="car", annotations=1, assets=1),) + assert fixture.releases.list(fixture.dataset_id) == [] + fixture.close() + + +def test_release_publication_refuses_an_attribute_removed_from_the_active_class( + tmp_path: Path, +) -> None: + fixture = Fixture(tmp_path) + asset_id = fixture.asset_ids[0] + car_with_attribute = LabelClass( + name="car", + geometries=(GeometryType.BBOX,), + attributes=(Attribute(name="occluded", kind="boolean"),), + ) + _promote_under_superseded_schema( + fixture, + [SIGN, car_with_attribute], + [SIGN, CAR], + Annotation( + asset_id=asset_id, + label_class="car", + schema_version=1, + geometry=BboxGeometry(x=1.0, y=2.0, width=30.0, height=40.0), + attributes={"occluded": False}, + provenance="human", + ), + ) + + with pytest.raises(ReleaseContentWouldViolateSchema) as caught: + fixture.releases.publish(fixture.dataset_id, "v1") + + assert caught.value.blockers == (ClassCount(label_class="car", annotations=1, assets=1),) + assert fixture.releases.list(fixture.dataset_id) == [] + fixture.close() + + def test_publishing_twice_from_an_unchanged_dataset_reuses_the_one_manifest_blob( tmp_path: Path, ) -> None: @@ -183,6 +293,51 @@ def test_publishing_twice_from_an_unchanged_dataset_reuses_the_one_manifest_blob fixture.close() +def test_reading_a_legacy_manifest_with_an_undeclared_annotation_does_not_reapply_the_gate( + tmp_path: Path, +) -> None: + fixture = Fixture(tmp_path) + release = fixture.releases.publish(fixture.ready(), "v1") + manifest = fixture.releases.manifest(release.id) + asset = manifest.assets[0] + legacy_manifest = manifest.model_copy( + update={ + "assets": ( + asset.model_copy( + update={ + "annotations": ( + ManifestAnnotation( + id=uuid4(), + label_class="car", + schema_version=1, + geometry=BboxGeometry(x=1.0, y=2.0, width=30.0, height=40.0), + provenance="human", + ), + ) + } + ), + ) + } + ) + manifest_hash = fixture.workspace.blob_store.put(BytesIO(canonical_bytes(legacy_manifest))) + with fixture.workspace.unit_of_work() as uow: + legacy = uow.releases.add( + Release( + dataset_id=fixture.dataset_id, + tag="legacy", + manifest_hash=manifest_hash, + schema_version=legacy_manifest.schema_version, + asset_count=len(legacy_manifest.assets), + annotation_count=legacy_manifest.annotation_count, + visionset_version=__version__, + ) + ) + + assert fixture.releases.manifest(legacy.id) == legacy_manifest + assert fixture.releases.verify(legacy.id).ok + fixture.close() + + def test_a_release_records_the_date_and_the_visionset_version_that_made_it( tmp_path: Path, ) -> None: diff --git a/tests/kernel/test_schema_validation.py b/tests/kernel/test_schema_validation.py new file mode 100644 index 00000000..d994b451 --- /dev/null +++ b/tests/kernel/test_schema_validation.py @@ -0,0 +1,107 @@ +"""Schema-dependent annotation validation without workspace state.""" + +from __future__ import annotations + +from collections.abc import Mapping +from uuid import uuid4 + +import pytest + +from visionset.kernel import ( + DisallowedGeometry, + InvalidAnnotation, + InvalidAttributeValue, + LabelClassNotInSchema, + MissingRequiredAttribute, + UnknownAttribute, +) +from visionset.kernel.domain import ( + AnnotationSchema, + Attribute, + AttributeValue, + BboxGeometry, + Geometry, + GeometryType, + LabelClass, + PolygonGeometry, + validate_schema_annotation, +) + +CAR = LabelClass( + name="car", + geometries=(GeometryType.BBOX,), + attributes=(Attribute(name="occluded", kind="boolean", required=True),), +) +SCHEMA = AnnotationSchema(project_id=uuid4(), version=2, classes=(CAR,)) +TRIANGLE = [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0)] + + +@pytest.mark.parametrize( + ("label_class", "geometry", "attributes", "error"), + [ + pytest.param( + "car", + BboxGeometry(x=1.0, y=2.0, width=3.0, height=4.0), + {"occluded": False}, + None, + id="accepts-a-matching-annotation", + ), + pytest.param( + "truck", + BboxGeometry(x=1.0, y=2.0, width=3.0, height=4.0), + {"occluded": False}, + LabelClassNotInSchema, + id="refuses-a-class-outside-the-schema", + ), + pytest.param( + "car", + PolygonGeometry(points=TRIANGLE), + {"occluded": False}, + DisallowedGeometry, + id="refuses-a-geometry-the-class-does-not-allow", + ), + pytest.param( + "car", + BboxGeometry(x=1.0, y=2.0, width=3.0, height=4.0), + {"occluded": False, "colour": "red"}, + UnknownAttribute, + id="refuses-an-attribute-the-class-does-not-declare", + ), + pytest.param( + "car", + BboxGeometry(x=1.0, y=2.0, width=3.0, height=4.0), + {}, + MissingRequiredAttribute, + id="refuses-a-missing-required-attribute", + ), + pytest.param( + "car", + BboxGeometry(x=1.0, y=2.0, width=3.0, height=4.0), + {"occluded": "yes"}, + InvalidAttributeValue, + id="refuses-an-attribute-value-of-the-wrong-type", + ), + ], +) +def test_a_schema_validator_reuses_the_annotation_write_rules( + label_class: str, + geometry: Geometry, + attributes: Mapping[str, AttributeValue], + error: type[InvalidAnnotation] | None, +) -> None: + """A change that skips any schema rule makes its corresponding row pass unexpectedly.""" + if error is None: + validate_schema_annotation( + label_class=label_class, + geometry=geometry, + attributes=attributes, + schema=SCHEMA, + ) + else: + with pytest.raises(error): + validate_schema_annotation( + label_class=label_class, + geometry=geometry, + attributes=attributes, + schema=SCHEMA, + ) diff --git a/tests/mcp/test_release_tools.py b/tests/mcp/test_release_tools.py index 11bc1b97..ba607288 100644 --- a/tests/mcp/test_release_tools.py +++ b/tests/mcp/test_release_tools.py @@ -113,6 +113,50 @@ def test_publishing_an_empty_dataset_is_refused( assert error(call("publish_release", project=named, tag="v1.0"))["message"] +def test_publishing_refuses_content_the_active_schema_no_longer_describes( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + car = {"name": "car", "geometries": ["bbox"]} + named, batch_id, job_id = open_batch( + monkeypatch, tmp_path, count=1, classes=[*SCHEMA_CLASSES, car] + ) + payload( + call( + "create_schema_version", + project=named, + classes=SCHEMA_CLASSES, + allow_destructive=True, + ) + ) + asset_id = payload(call("next_pending_assets", job_id=job_id, count=1))["items"][0]["id"] + payload( + call( + "add_annotations", + job_id=job_id, + annotations=[ + { + "asset_id": asset_id, + "label_class": "car", + "geometry": BBOX, + "provenance": "model", + "model_ref": "probe@1", + } + ], + ) + ) + payload(call("complete_job", job_id=job_id)) + payload(call("complete_batch", batch_id=batch_id)) + payload(call("promote_batch", batch_id=batch_id)) + + refusal = error(call("publish_release", project=named, tag="v1.0")) + + assert refusal["retry_with"] is None + assert ( + "reconcile the annotations or publish a schema that describes them" + in refusal["hint"].lower() + ) + + def test_a_release_tag_is_case_sensitive_unlike_a_project_name( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/server/test_errors.py b/tests/server/test_errors.py index 32bb4f59..87b6d48c 100644 --- a/tests/server/test_errors.py +++ b/tests/server/test_errors.py @@ -94,6 +94,7 @@ "JobNotComplete": (409, "JOB_NOT_COMPLETE"), "EmptyBatch": (409, "EMPTY_BATCH"), "EmptyRelease": (409, "EMPTY_RELEASE"), + "ReleaseContentWouldViolateSchema": (409, "RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA"), "ConfirmationRequired": (409, "CONFIRMATION_REQUIRED"), "DestructiveSchemaChange": (409, "DESTRUCTIVE_SCHEMA_CHANGE"), "SchemaChangeWouldOrphan": (409, "SCHEMA_CHANGE_WOULD_ORPHAN"), diff --git a/tests/server/test_releases.py b/tests/server/test_releases.py index 95e60444..ed359cde 100644 --- a/tests/server/test_releases.py +++ b/tests/server/test_releases.py @@ -32,7 +32,14 @@ reset_exporters, with_exporters, ) -from tests.server._flow import dataset_of, promoted_dataset +from tests.server._flow import ( + SIGN, + asset_ids, + batch_from_ingest, + dataset_of, + project_with_schema, + promoted_dataset, +) from tests.server._jobs import InlineDispatcher from visionset.kernel.domain import MANIFEST_VERSION @@ -152,6 +159,47 @@ def test_publishing_an_empty_trunk_is_409(client: TestClient, tmp_path: Path) -> assert response.json()["code"] == "EMPTY_RELEASE" +def test_release_publication_reports_active_schema_content_blockers( + client: TestClient, runner: InlineDispatcher, tmp_path: Path +) -> None: + project_id = project_with_schema( + client, classes=[SIGN, {"name": "car", "geometries": ["bbox"]}] + ) + batch_id = batch_from_ingest(client, runner, tmp_path, project_id, images=1) + client.post(f"/batches/{batch_id}/approve") + client.post(f"/batches/{batch_id}/start") + job_id = client.get(f"/batches/{batch_id}/jobs").json()["items"][0]["id"] + client.post(f"/jobs/{job_id}/start") + client.post( + f"/projects/{project_id}/schema/versions", + json={"classes": [SIGN]}, + params={"allow_destructive": True}, + ) + client.post( + f"/jobs/{job_id}/annotations", + json=[ + { + "asset_id": asset_ids(client, batch_id)[0], + "label_class": "car", + "geometry": {"type": "bbox", "x": 1, "y": 2, "width": 30, "height": 40}, + "provenance": "human", + } + ], + ) + client.post(f"/jobs/{job_id}/complete") + client.post(f"/batches/{batch_id}/complete") + client.post(f"/batches/{batch_id}/promote") + dataset_id = dataset_of(client, project_id) + + response = client.post(f"/datasets/{dataset_id}/releases", json={"tag": "v1"}) + + assert response.status_code == 409 + body = response.json() + assert body["code"] == "RELEASE_CONTENT_WOULD_VIOLATE_SCHEMA" + assert body["detail"] == {"blockers": [{"label_class": "car", "annotations": 1, "assets": 1}]} + assert client.get(f"/datasets/{dataset_id}/releases").json() == {"items": [], "total": 0} + + def test_publishing_from_an_unknown_dataset_is_404(client: TestClient) -> None: response = client.post(f"/datasets/{uuid4()}/releases", json={"tag": "v1"})