diff --git a/docs/api.md b/docs/api.md index 18357631..9c85d564 100644 --- a/docs/api.md +++ b/docs/api.md @@ -582,7 +582,7 @@ argument for branching on `code`. | **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` | -| **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` · `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` | +| **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 2592fcbd..68194834 100644 --- a/docs/batches.md +++ b/docs/batches.md @@ -330,8 +330,10 @@ text-prompted detector answers with text decoded from spans over that prompt rat choice from the list, so an answer naming one of the classes, matched case-insensitively, is written under the schema's own spelling, and an answer naming none of them - a span that crossed the boundary between two phrases, most often - is discarded rather than guessed onto -either half; the outcome's `regions_discarded` says how many. A schema with no such class is -refused up front; see [inference.md](inference.md#what-a-connection-can-be-asked-for). +either half; the outcome's `regions_discarded` says how many. A mapped region whose geometry has +no overlap with a measured asset is discarded separately, and `regions_out_of_bounds` says how +many; unmeasured assets remain eligible. A schema with no such class is refused up front; see +[inference.md](inference.md#what-a-connection-can-be-asked-for). **What lands enters at `pre_labeled`, never `annotated`.** Nobody judged it, so it arrives in its own editable state rather than claiming to be somebody's work - see @@ -360,8 +362,9 @@ most recent `annotation.pre_label` job naming this batch - live or settled - and reload, a second tab or a run started at a terminal can only be shown by the batch itself saying so. Counted in assets, the unit this handler works in, and carrying the outcome `prelabel.py`'s `run` returns once the job has settled - `stopped_early`, `assets_labeled`, -`regions_discarded` - so a client can tell a cancelled run from an untouched batch. Derived, never -stored, and published on `BatchOut` as `pre_label_run`, `null` where none ever ran. +`regions_discarded`, `regions_out_of_bounds` - so a client can tell a cancelled run from an +untouched batch. Derived, never stored, and published on `BatchOut` as `pre_label_run`, `null` +where none ever ran. ## What approval and completion announce diff --git a/docs/cli.md b/docs/cli.md index 40c70f2a..230051a0 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -360,7 +360,8 @@ interrupted run. The batch id goes to stdout. `pre-label BATCH_ID CONNECTION [--minimum-confidence FLOAT]` blocks and calls `visionset.inference.pre_label` inline because a terminal has no dispatcher. Progress and the summary are written to stderr; normal stdout contains `annotations_written`. With `--json`, the -command prints the complete outcome instead. +command prints the complete outcome instead, including `regions_discarded` for unmappable model +labels and `regions_out_of_bounds` for mapped regions without overlap with a measured asset. `--jobs-of N` is the `BySize` partition; with no flag the batch becomes one job. There is no `batch create` and no membership editing: a batch is born from an ingest. See diff --git a/docs/mcp.md b/docs/mcp.md index 6bac9589..b31be794 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -306,6 +306,9 @@ retry resumes the cache rather than starting over; a cut-off pre-labeling call h the assets it fully entered, one commit per asset, so calling it again resumes with whatever is still untouched. +`pre_label_batch` reports unmappable model labels as `regions_discarded` and mapped regions +without overlap with a measured asset as `regions_out_of_bounds`. + There is therefore no ingest polling, and no `resume_ingest`. If a call is cut off part way, call `ingest` again - registration is idempotent on `(kind, path, extraction_fps)` and content addressing means the re-run creates nothing it created before. That is the same argument that diff --git a/frontend/ui-core/src/generated/api.ts b/frontend/ui-core/src/generated/api.ts index 239503b9..2085acc6 100644 --- a/frontend/ui-core/src/generated/api.ts +++ b/frontend/ui-core/src/generated/api.ts @@ -3063,9 +3063,9 @@ export interface components { * @description An axis-aligned rectangle: top-left corner plus size. * * ``width`` and ``height`` must be strictly positive — a zero-area box is as - * meaningless as a negative one, so neither is accepted. ``x`` and ``y`` are - * unconstrained: an annotation may legitimately start outside the asset's - * bounds when an object is clipped by the frame edge. + * meaningless as a negative one, so neither is accepted. A box may extend + * beyond an asset's frame, but cannot be wholly disjoint when that asset + * records its dimensions. */ BboxGeometry: { /** Height */ @@ -3961,8 +3961,9 @@ export interface components { * set before the first forward pass, so both its progress and its total are * counted in the unit its own work is over. * - * **The outcome, once the job has one.** `stopped_early`, `assets_labeled` and - * `regions_discarded` are the handler's own account of what a settled run did. + * **The outcome, once the job has one.** `stopped_early`, `assets_labeled`, + * `regions_discarded` and `regions_out_of_bounds` are the handler's own + * account of what a settled run did. * They are `null` while the job is still `queued` or `running`, and `null` * where it ended `failed` before producing one — but a `cancelled` run still * carries them: stopping partway is a coherent outcome for a handler whose @@ -3984,6 +3985,8 @@ export interface components { job_id: string; /** Regions Discarded */ regions_discarded: number | null; + /** Regions Out Of Bounds */ + regions_out_of_bounds: number | null; state: components["schemas"]["BackgroundJobState"]; /** Stopped Early */ stopped_early: boolean | null; diff --git a/frontend/ui-core/src/generated/checks.ts b/frontend/ui-core/src/generated/checks.ts index 38f2c1da..8b29bb88 100644 --- a/frontend/ui-core/src/generated/checks.ts +++ b/frontend/ui-core/src/generated/checks.ts @@ -100,7 +100,7 @@ export const checkBatchState: Check = /*#__PURE__*/ oneOf(["draft", "approved", "in_annotation", "completed"] as const); export const checkPreLabelRunOut: Check = - /*#__PURE__*/ object({ "assets_labeled": [true, either([isInteger, isNull] as const)], "assets_processed": [true, isInteger], "assets_total": [true, either([isInteger, isNull] as const)], "error": [true, either([isString, isNull] as const)], "job_id": [true, isString], "regions_discarded": [true, either([isInteger, isNull] as const)], "state": [true, checkBackgroundJobState], "stopped_early": [true, either([isBoolean, isNull] as const)] } as const); + /*#__PURE__*/ object({ "assets_labeled": [true, either([isInteger, isNull] as const)], "assets_processed": [true, isInteger], "assets_total": [true, either([isInteger, isNull] as const)], "error": [true, either([isString, isNull] as const)], "job_id": [true, isString], "regions_discarded": [true, either([isInteger, isNull] as const)], "regions_out_of_bounds": [true, either([isInteger, isNull] as const)], "state": [true, checkBackgroundJobState], "stopped_early": [true, either([isBoolean, isNull] as const)] } as const); export const checkProgressCounts: Check = /*#__PURE__*/ object({ "accepted": [true, isInteger], "annotated": [true, isInteger], "pre_labeled": [true, isInteger], "review_pending": [true, isInteger], "skipped": [true, isInteger], "total": [true, isInteger], "unannotated": [true, isInteger] } as const); diff --git a/frontend/ui-core/src/screens/PreLabelDialog.tsx b/frontend/ui-core/src/screens/PreLabelDialog.tsx index 24502050..7d3e0ba3 100644 --- a/frontend/ui-core/src/screens/PreLabelDialog.tsx +++ b/frontend/ui-core/src/screens/PreLabelDialog.tsx @@ -137,6 +137,7 @@ interface RunView { readonly stoppedEarly: boolean | null; readonly assetsLabeled: number | null; readonly regionsDiscarded: number | null; + readonly regionsOutOfBounds: number | null; } /** @@ -148,6 +149,7 @@ function viewFromJob(job: BackgroundJob): RunView { const stoppedEarly = job.result.stopped_early; const assetsLabeled = job.result.assets_labeled; const regionsDiscarded = job.result.regions_discarded; + const regionsOutOfBounds = job.result.regions_out_of_bounds; return { jobId: job.id, state: job.state, @@ -157,6 +159,7 @@ function viewFromJob(job: BackgroundJob): RunView { stoppedEarly: typeof stoppedEarly === "boolean" ? stoppedEarly : null, assetsLabeled: typeof assetsLabeled === "number" ? assetsLabeled : null, regionsDiscarded: typeof regionsDiscarded === "number" ? regionsDiscarded : null, + regionsOutOfBounds: typeof regionsOutOfBounds === "number" ? regionsOutOfBounds : null, }; } @@ -171,6 +174,7 @@ function viewFromRun(run: PreLabelRun): RunView { stoppedEarly: run.stopped_early, assetsLabeled: run.assets_labeled, regionsDiscarded: run.regions_discarded, + regionsOutOfBounds: run.regions_out_of_bounds, }; } @@ -223,25 +227,36 @@ function failedProgress(view: RunView): string | null { } /** What a settled run actually did, in words — including the one count no other UI shows. */ -function DoneSummary({ result }: { readonly result: BackgroundJob["result"] }): JSX.Element { - const labeled = typeof result.assets_labeled === "number" ? result.assets_labeled : 0; - const written = typeof result.annotations_written === "number" ? result.annotations_written : 0; - const discarded = typeof result.regions_discarded === "number" ? result.regions_discarded : 0; - const skipped = typeof result.assets_skipped === "number" ? result.assets_skipped : 0; - const stoppedEarly = result.stopped_early === true; +function DoneSummary({ + view, + result, +}: { + readonly view: RunView; + readonly result: BackgroundJob["result"] | null; +}): JSX.Element { + const labeled = typeof result?.assets_labeled === "number" ? result.assets_labeled : 0; + const written = typeof result?.annotations_written === "number" ? result.annotations_written : 0; + const discarded = view.regionsDiscarded ?? 0; + const outOfBounds = view.regionsOutOfBounds ?? 0; + const skipped = typeof result?.assets_skipped === "number" ? result.assets_skipped : 0; + const stoppedEarly = result?.stopped_early === true; return (
-

- Labeled {labeled} asset{labeled === 1 ? "" : "s"}, writing {written} region - {written === 1 ? "" : "s"} for you to correct. -

+ {result !== null && ( +

+ Labeled {labeled} asset{labeled === 1 ? "" : "s"}, writing {written} region + {written === 1 ? "" : "s"} for you to correct. +

+ )} {discarded > 0 && (

- Discarded {discarded} region{discarded === 1 ? "" : "s"} for naming a class the - prompt did not ask for. + {discarded} model region{discarded === 1 ? " did" : "s did"} not match a requested class.

)} + {outOfBounds > 0 && ( +

{`${outOfBounds} model region${outOfBounds === 1 ? " was" : "s were"} outside their asset and were skipped.`}

+ )} {skipped > 0 && (

Skipped {skipped} asset{skipped === 1 ? "" : "s"} that work had already started on @@ -510,7 +525,9 @@ function PreLabelDialog({

)} - {mode === "done" && launched !== null && } + {mode === "done" && view !== null && ( + + )} {mode === "stopped" && view !== null && (

diff --git a/frontend/ui-core/src/screens/preLabel.test.tsx b/frontend/ui-core/src/screens/preLabel.test.tsx index 1afe99dc..79c0e50b 100644 --- a/frontend/ui-core/src/screens/preLabel.test.tsx +++ b/frontend/ui-core/src/screens/preLabel.test.tsx @@ -168,6 +168,7 @@ function preLabelRunOf(overrides: Record = {}): Record { +it("separates unmappable and out-of-bounds model regions in a completed job", async () => { on("GET", /\/background-jobs\//, { status: 200, body: backgroundJobOf({ @@ -471,6 +472,7 @@ it("says how many regions a run discarded, in words, when it discarded any", asy assets_labeled: 40, annotations_written: 90, regions_discarded: 3, + regions_out_of_bounds: 2, assets_skipped: 0, stopped_early: false, }, @@ -481,8 +483,9 @@ it("says how many regions a run discarded, in words, when it discarded any", asy await userEvent.click(await screen.findByRole("button", { name: /pre-label/i })); await userEvent.click(await screen.findByRole("button", { name: /start/i })); - const discarded = await screen.findByTestId("prelabel-discarded"); - expect(discarded.textContent).toMatch(/3 regions/); + const summary = await screen.findByTestId("prelabel-summary"); + expect(summary.textContent).toContain("3 model regions did not match a requested class"); + expect(summary.textContent).toContain("2 model regions were outside their asset and were skipped"); }); it("says nothing about discarded regions when a run discarded none", async () => { @@ -494,6 +497,7 @@ it("says nothing about discarded regions when a run discarded none", async () => assets_labeled: 40, annotations_written: 90, regions_discarded: 0, + regions_out_of_bounds: 0, assets_skipped: 0, stopped_early: false, }, @@ -507,6 +511,8 @@ it("says nothing about discarded regions when a run discarded none", async () => await screen.findByRole("button", { name: /edit these frames/i }); expect(screen.queryByTestId("prelabel-discarded")).toBeNull(); expect(screen.queryByText(/^0$/)).toBeNull(); + expect(screen.queryByText(/did not match a requested class/i)).toBeNull(); + expect(screen.queryByText(/outside their asset and were skipped/i)).toBeNull(); }); /** @@ -556,6 +562,7 @@ it("on reopen after a failed run, shows the handler's error and offers Try again stopped_early: null, assets_labeled: null, regions_discarded: null, + regions_out_of_bounds: null, }), }, { counts: { unannotated: 43, total: 48 } }, @@ -583,6 +590,7 @@ it("on reopen after a complete run, disables Start with its reason adjacent and stopped_early: false, assets_labeled: 48, regions_discarded: 0, + regions_out_of_bounds: 2, }), }, { counts: { unannotated: 0, total: 48 } }, @@ -594,6 +602,9 @@ it("on reopen after a complete run, disables Start with its reason adjacent and expect(start.disabled).toBe(true); const reason = await screen.findByTestId("prelabel-blocked-reason"); expect(reason.textContent).toMatch(/pre-labeled/i); + expect((await screen.findByTestId("prelabel-summary")).textContent).toContain( + "2 model regions were outside their asset and were skipped", + ); const edit = await screen.findByRole("button", { name: /edit these frames/i }); await userEvent.click(edit); diff --git a/openapi.json b/openapi.json index 8b0810a3..c6a01d27 100644 --- a/openapi.json +++ b/openapi.json @@ -1445,7 +1445,7 @@ }, "BboxGeometry": { "additionalProperties": false, - "description": "An axis-aligned rectangle: top-left corner plus size.\n\n``width`` and ``height`` must be strictly positive \u2014 a zero-area box is as\nmeaningless as a negative one, so neither is accepted. ``x`` and ``y`` are\nunconstrained: an annotation may legitimately start outside the asset's\nbounds when an object is clipped by the frame edge.", + "description": "An axis-aligned rectangle: top-left corner plus size.\n\n``width`` and ``height`` must be strictly positive \u2014 a zero-area box is as\nmeaningless as a negative one, so neither is accepted. A box may extend\nbeyond an asset's frame, but cannot be wholly disjoint when that asset\nrecords its dimensions.", "properties": { "height": { "exclusiveMinimum": 0.0, @@ -3185,7 +3185,7 @@ "type": "object" }, "PreLabelRunOut": { - "description": "A batch's most recent pre-labeling run: which job, how far, and what it found.\n\nPresent whenever pre-labeling has ever been asked for on this batch, and\ndescribing the most recent run \u2014 including one this session did not launch.\nA dialog reopened after a reload, in a second tab, or after a run started\nfrom the terminal reads the same state from here rather than from a job id\na component happened to keep.\n\n**Assets, where a download counts bytes and a check counts files.** The\nhandler owns a loop over the batch's untouched assets and knows the whole\nset before the first forward pass, so both its progress and its total are\ncounted in the unit its own work is over.\n\n**The outcome, once the job has one.** `stopped_early`, `assets_labeled` and\n`regions_discarded` are the handler's own account of what a settled run did.\nThey are `null` while the job is still `queued` or `running`, and `null`\nwhere it ended `failed` before producing one \u2014 but a `cancelled` run still\ncarries them: stopping partway is a coherent outcome for a handler whose\ncontract is to write only where nothing has been written.", + "description": "A batch's most recent pre-labeling run: which job, how far, and what it found.\n\nPresent whenever pre-labeling has ever been asked for on this batch, and\ndescribing the most recent run \u2014 including one this session did not launch.\nA dialog reopened after a reload, in a second tab, or after a run started\nfrom the terminal reads the same state from here rather than from a job id\na component happened to keep.\n\n**Assets, where a download counts bytes and a check counts files.** The\nhandler owns a loop over the batch's untouched assets and knows the whole\nset before the first forward pass, so both its progress and its total are\ncounted in the unit its own work is over.\n\n**The outcome, once the job has one.** `stopped_early`, `assets_labeled`,\n`regions_discarded` and `regions_out_of_bounds` are the handler's own\naccount of what a settled run did.\nThey are `null` while the job is still `queued` or `running`, and `null`\nwhere it ended `failed` before producing one \u2014 but a `cancelled` run still\ncarries them: stopping partway is a coherent outcome for a handler whose\ncontract is to write only where nothing has been written.", "properties": { "assets_labeled": { "anyOf": [ @@ -3240,6 +3240,17 @@ ], "title": "Regions Discarded" }, + "regions_out_of_bounds": { + "anyOf": [ + { + "type": "integer" + }, + { + "type": "null" + } + ], + "title": "Regions Out Of Bounds" + }, "state": { "$ref": "#/components/schemas/BackgroundJobState" }, @@ -3263,7 +3274,8 @@ "error", "stopped_early", "assets_labeled", - "regions_discarded" + "regions_discarded", + "regions_out_of_bounds" ], "title": "PreLabelRunOut", "type": "object" diff --git a/src/visionset/inference/prelabel.py b/src/visionset/inference/prelabel.py index 76742b05..26f83a25 100644 --- a/src/visionset/inference/prelabel.py +++ b/src/visionset/inference/prelabel.py @@ -46,6 +46,7 @@ TextPrompt, media_type_of, ) +from visionset.kernel.domain.geometry import geometry_intersects_asset from visionset.kernel.errors import ( AssetNotWritable, SchemaHasNoDetectableClass, @@ -95,6 +96,9 @@ class PreLabelOutcome: #: a meaningful share of the model's output says so instead of reporting a #: clean success. regions_discarded: int = 0 + #: Regions whose geometry has no overlap with a measured asset. Kept + #: separate from unmappable labels because their class mapping succeeded. + regions_out_of_bounds: int = 0 def unsupported_prompt_message(connection_name: str) -> str: @@ -202,6 +206,10 @@ def pre_label( the phrase list, and a merged answer is discarded rather than guessed onto either half. ``PreLabelOutcome.regions_discarded`` says how many. + A region whose mapped geometry has no overlap with a measured asset is also + passed over before the atomic write. ``PreLabelOutcome.regions_out_of_bounds`` + says how many; unmeasured assets remain eligible. + Raises: InferenceConnectionNotFound: no such connection. InferenceConnectionNotSetUp: a local connection whose weights are absent. @@ -234,7 +242,7 @@ def pre_label( annotations_service = AnnotationService(workspace) ingest = IngestService(workspace) - considered = labeled = written = skipped = discarded = 0 + considered = labeled = written = skipped = discarded = out_of_bounds = 0 model_ref: str | None = None for job_id, asset_id in targets: # Between assets, which is the only place stopping is honest: the last @@ -248,6 +256,7 @@ def pre_label( stopped_early=True, assets_skipped=skipped, regions_discarded=discarded, + regions_out_of_bounds=out_of_bounds, ) asset = ingest.asset(batch.project_id, asset_id) @@ -273,9 +282,17 @@ def pre_label( class_by_answer=class_by_answer, ) discarded += unmapped - if proposed: + in_bounds = [ + annotation + for annotation in proposed + if geometry_intersects_asset( + annotation.geometry, width=asset.width, height=asset.height + ) + ] + out_of_bounds += len(proposed) - len(in_bounds) + if in_bounds: try: - annotations_service.enter_unreviewed(job_id, proposed) + annotations_service.enter_unreviewed(job_id, in_bounds) except AssetNotWritable: # The batch is `in_annotation`, so somebody working in it # while this run is in flight is the normal case, not a @@ -287,12 +304,18 @@ def pre_label( skipped += 1 else: labeled += 1 - written += len(proposed) + written += len(in_bounds) if on_progress is not None: on_progress(considered, total) return PreLabelOutcome( - considered, labeled, written, model_ref, assets_skipped=skipped, regions_discarded=discarded + considered, + labeled, + written, + model_ref, + assets_skipped=skipped, + regions_discarded=discarded, + regions_out_of_bounds=out_of_bounds, ) diff --git a/src/visionset/jobs/prelabel.py b/src/visionset/jobs/prelabel.py index 782da71d..5a42fd11 100644 --- a/src/visionset/jobs/prelabel.py +++ b/src/visionset/jobs/prelabel.py @@ -84,6 +84,10 @@ def run( rather than a choice from the prompt's phrases, so a merged answer is dropped before it is ever written; ``regions_discarded`` in the result says how many. + + **A mapped region with no overlap with a measured asset is also discarded.** + ``regions_out_of_bounds`` keeps that geometry refusal distinct from an + unmappable model label. """ if reporter.is_cancelled(): return {} @@ -110,4 +114,5 @@ def run( "stopped_early": outcome.stopped_early, "assets_skipped": outcome.assets_skipped, "regions_discarded": outcome.regions_discarded, + "regions_out_of_bounds": outcome.regions_out_of_bounds, } diff --git a/src/visionset/kernel/__init__.py b/src/visionset/kernel/__init__.py index d263fc10..d299d6ea 100644 --- a/src/visionset/kernel/__init__.py +++ b/src/visionset/kernel/__init__.py @@ -7,6 +7,7 @@ """ from visionset.kernel.errors import ( + AnnotationGeometryOutOfBounds, AnnotationNotFound, AnnotationNotFromModel, AssetNotFound, @@ -90,6 +91,7 @@ ) __all__ = [ + "AnnotationGeometryOutOfBounds", "AnnotationNotFound", "AnnotationNotFromModel", "AssetNotFound", diff --git a/src/visionset/kernel/domain/geometry.py b/src/visionset/kernel/domain/geometry.py index 11269a24..7c5c55bc 100644 --- a/src/visionset/kernel/domain/geometry.py +++ b/src/visionset/kernel/domain/geometry.py @@ -18,6 +18,8 @@ from __future__ import annotations +from dataclasses import dataclass +from math import isfinite from typing import Annotated, Final, Literal, get_args from pydantic import BaseModel, ConfigDict, Field, model_validator @@ -29,9 +31,9 @@ class BboxGeometry(BaseModel): """An axis-aligned rectangle: top-left corner plus size. ``width`` and ``height`` must be strictly positive — a zero-area box is as - meaningless as a negative one, so neither is accepted. ``x`` and ``y`` are - unconstrained: an annotation may legitimately start outside the asset's - bounds when an object is clipped by the frame edge. + meaningless as a negative one, so neither is accepted. A box may extend + beyond an asset's frame, but cannot be wholly disjoint when that asset + records its dimensions. """ model_config = ConfigDict(frozen=True, extra="forbid") @@ -133,3 +135,140 @@ class ClassificationGeometry(BaseModel): ``SchemaService`` checks a proposed ``LabelClass`` against: declaring a class whose geometry has no model would create a class nobody could ever annotate. """ + + +@dataclass(frozen=True) +class _Frame: + width: float + height: float + + +def geometry_intersects_asset(geometry: Geometry, *, width: int | None, height: int | None) -> bool: + if width is None or height is None or not _coordinates_are_finite(geometry): + return True + frame = _Frame(width=float(width), height=float(height)) + if isinstance(geometry, ClassificationGeometry): + return True + if isinstance(geometry, BboxGeometry): + return _bbox_intersects_frame(geometry, frame) + if isinstance(geometry, PolylineGeometry): + return _path_intersects_frame(geometry.points, frame, closed=False) + return _path_intersects_frame(geometry.points, frame, closed=True) + + +def _coordinates_are_finite(geometry: Geometry) -> bool: + if isinstance(geometry, ClassificationGeometry): + return True + if isinstance(geometry, BboxGeometry): + return all( + isfinite(value) for value in (geometry.x, geometry.y, geometry.width, geometry.height) + ) + return all(isfinite(value) for point in geometry.points for value in point) + + +def _bbox_intersects_frame(geometry: BboxGeometry, frame: _Frame) -> bool: + return ( + geometry.x <= frame.width + and geometry.x + geometry.width >= 0.0 + and geometry.y <= frame.height + and geometry.y + geometry.height >= 0.0 + ) + + +def _path_intersects_frame( + points: list[tuple[float, float]], frame: _Frame, *, closed: bool +) -> bool: + if any(_point_is_in_frame(point, frame) for point in points): + return True + + frame_edges = _frame_edges(frame) + segments = list(zip(points, points[1:], strict=False)) + if closed: + segments.append((points[-1], points[0])) + if any( + _segments_intersect(start, end, edge_start, edge_end) + for start, end in segments + for edge_start, edge_end in frame_edges + ): + return True + + return closed and any(_point_is_in_polygon(corner, points) for corner in _frame_corners(frame)) + + +def _point_is_in_frame(point: tuple[float, float], frame: _Frame) -> bool: + x, y = point + return 0.0 <= x <= frame.width and 0.0 <= y <= frame.height + + +def _frame_edges( + frame: _Frame, +) -> tuple[ + tuple[tuple[float, float], tuple[float, float]], + tuple[tuple[float, float], tuple[float, float]], + tuple[tuple[float, float], tuple[float, float]], + tuple[tuple[float, float], tuple[float, float]], +]: + top_left = (0.0, 0.0) + top_right = (frame.width, 0.0) + bottom_right = (frame.width, frame.height) + bottom_left = (0.0, frame.height) + return ( + (top_left, top_right), + (top_right, bottom_right), + (bottom_right, bottom_left), + (bottom_left, top_left), + ) + + +def _frame_corners(frame: _Frame) -> tuple[tuple[float, float], ...]: + return ((0.0, 0.0), (frame.width, 0.0), (frame.width, frame.height), (0.0, frame.height)) + + +def _segments_intersect( + first_start: tuple[float, float], + first_end: tuple[float, float], + second_start: tuple[float, float], + second_end: tuple[float, float], +) -> bool: + first_second_start = _cross_product(first_start, first_end, second_start) + first_second_end = _cross_product(first_start, first_end, second_end) + second_first_start = _cross_product(second_start, second_end, first_start) + second_first_end = _cross_product(second_start, second_end, first_end) + + if first_second_start == 0.0 and _point_is_on_segment(second_start, first_start, first_end): + return True + if first_second_end == 0.0 and _point_is_on_segment(second_end, first_start, first_end): + return True + if second_first_start == 0.0 and _point_is_on_segment(first_start, second_start, second_end): + return True + if second_first_end == 0.0 and _point_is_on_segment(first_end, second_start, second_end): + return True + return (first_second_start > 0.0) != (first_second_end > 0.0) and ( + second_first_start > 0.0 + ) != (second_first_end > 0.0) + + +def _point_is_in_polygon(point: tuple[float, float], points: list[tuple[float, float]]) -> bool: + winding_number = 0 + for start, end in zip(points, [*points[1:], points[0]], strict=True): + if _cross_product(start, end, point) == 0.0 and _point_is_on_segment(point, start, end): + return True + if start[1] <= point[1] < end[1] and _cross_product(start, end, point) > 0.0: + winding_number += 1 + elif end[1] <= point[1] < start[1] and _cross_product(start, end, point) < 0.0: + winding_number -= 1 + return winding_number != 0 + + +def _point_is_on_segment( + point: tuple[float, float], start: tuple[float, float], end: tuple[float, float] +) -> bool: + return min(start[0], end[0]) <= point[0] <= max(start[0], end[0]) and min( + start[1], end[1] + ) <= point[1] <= max(start[1], end[1]) + + +def _cross_product( + start: tuple[float, float], end: tuple[float, float], point: tuple[float, float] +) -> float: + return (end[0] - start[0]) * (point[1] - start[1]) - (end[1] - start[1]) * (point[0] - start[0]) diff --git a/src/visionset/kernel/domain/inference.py b/src/visionset/kernel/domain/inference.py index 3b354045..231fb626 100644 --- a/src/visionset/kernel/domain/inference.py +++ b/src/visionset/kernel/domain/inference.py @@ -618,7 +618,8 @@ class PreLabelRun(BaseModel): is known, on ``ConnectionJob``'s own reasoning. **The handler's own outcome, carried rather than re-derived.** - ``stopped_early``, ``assets_labeled`` and ``regions_discarded`` are read + ``stopped_early``, ``assets_labeled``, ``regions_discarded`` and + ``regions_out_of_bounds`` are read straight out of the settled job's ``result`` — the dict ``prelabel.py``'s ``run`` returns — because a bare progress count cannot say how a cancelled run differs from an untouched batch, nor how many of a model's answers were @@ -658,6 +659,10 @@ class PreLabelRun(BaseModel): #: for, discarded rather than written. ``None`` until the job settles with a #: result. regions_discarded: int | None = None + #: Regions whose mapped geometry had no overlap with a measured asset, + #: discarded rather than written. ``None`` until the job settles with a + #: result. + regions_out_of_bounds: int | None = None @model_validator(mode="after") def _progress_is_within_its_total(self) -> Self: @@ -689,6 +694,7 @@ def of(cls, job: BackgroundJob) -> Self: stopped_early = result.get("stopped_early") assets_labeled = result.get("assets_labeled") regions_discarded = result.get("regions_discarded") + regions_out_of_bounds = result.get("regions_out_of_bounds") return cls( batch_id=UUID(named), job_id=job.id, @@ -697,11 +703,17 @@ def of(cls, job: BackgroundJob) -> Self: assets_total=job.total, error=job.error, stopped_early=stopped_early if isinstance(stopped_early, bool) else None, - assets_labeled=assets_labeled if isinstance(assets_labeled, int) else None, - regions_discarded=regions_discarded if isinstance(regions_discarded, int) else None, + assets_labeled=_result_int(assets_labeled), + regions_discarded=_result_int(regions_discarded), + regions_out_of_bounds=(_result_int(regions_out_of_bounds)), ) +def _result_int(value: object) -> int | None: + """An integer job-result value, without accepting Python's boolean subtype.""" + return value if isinstance(value, int) and not isinstance(value, bool) else None + + class InferenceConnection(BaseModel): """One configured place a model can be asked to predict. diff --git a/src/visionset/kernel/errors.py b/src/visionset/kernel/errors.py index 4dfe328a..d070811b 100644 --- a/src/visionset/kernel/errors.py +++ b/src/visionset/kernel/errors.py @@ -619,6 +619,10 @@ class DisallowedGeometry(InvalidAnnotation): """ +class AnnotationGeometryOutOfBounds(InvalidAnnotation): + """A spatial annotation has no overlap with its measured asset.""" + + class DuplicateClassificationTag(InvalidAnnotation): """This asset already carries a tag of this class. diff --git a/src/visionset/kernel/services/annotation_service.py b/src/visionset/kernel/services/annotation_service.py index 15ba01ac..d6bbd4fd 100644 --- a/src/visionset/kernel/services/annotation_service.py +++ b/src/visionset/kernel/services/annotation_service.py @@ -84,7 +84,9 @@ class is not in the version, whose geometry is not the one that class declares, ClassificationGeometry, progress_after_annotating, ) +from visionset.kernel.domain.geometry import geometry_intersects_asset from visionset.kernel.errors import ( + AnnotationGeometryOutOfBounds, AnnotationNotFound, AnnotationNotFromModel, AssetNotInJob, @@ -187,6 +189,7 @@ def add(self, job_id: UUID, annotations: Sequence[Annotation]) -> list[Annotatio with _blaming(index): _require_writable(job, annotation.asset_id) _validate(annotation, schema) + _require_geometry_on_asset(uow, annotation) # Checked inside this transaction and against a set that grows # as the loop goes, so a request carrying the same tag twice is # refused at the *second* position rather than by the index at @@ -247,6 +250,7 @@ def enter_unreviewed(self, job_id: UUID, annotations: Sequence[Annotation]) -> l _require_model_made(annotation) _require_untouched(uow, job, annotation.asset_id) _validate(annotation, schema) + _require_geometry_on_asset(uow, annotation) _require_untagged(tagged, annotation) stored = [uow.annotations.add(annotation) for annotation in proposed] @@ -304,6 +308,7 @@ def update(self, job_id: UUID, annotations: Sequence[Annotation]) -> list[Annota } ) _validate(replacement, schema) + _require_geometry_on_asset(uow, replacement) replacements.append(replacement) # After the loop, because an update may *move* a tag: the row being @@ -514,6 +519,20 @@ def _require_writable(job: AnnotationJob, asset_id: UUID) -> None: ) +def _require_geometry_on_asset(uow: UnitOfWork, annotation: Annotation) -> None: + """Refuse a spatial annotation wholly outside its measured asset.""" + asset = uow.assets.get(annotation.asset_id) + if asset is None: + raise WorkspaceCorrupt( + f"annotation {annotation.id} names asset {annotation.asset_id}, which does not exist" + ) + if not geometry_intersects_asset(annotation.geometry, width=asset.width, height=asset.height): + raise AnnotationGeometryOutOfBounds( + f"a {annotation.geometry.type.value} annotation on asset {asset.id} has no overlap " + f"with its {asset.width} by {asset.height} pixel frame" + ) + + def _require_model_made(annotation: Annotation) -> None: """Refuse a label this door was not built for.""" if annotation.provenance != "model": diff --git a/src/visionset/mcp/batches.py b/src/visionset/mcp/batches.py index 31d95f6f..e2b72420 100644 --- a/src/visionset/mcp/batches.py +++ b/src/visionset/mcp/batches.py @@ -317,6 +317,10 @@ def pre_label_batch( about, so a span crossing the boundary between two phrases can answer with neither of them; `regions_discarded` in the result says how many. + **A mapped region with no overlap with a measured asset is discarded + separately.** `regions_out_of_bounds` in the result says how many; an + asset without dimensions remains eligible. + **The batch's pinned schema is the prompt.** The model is asked for each class the schema declares that a box can be written as; an answer naming one of those classes, matched case-insensitively, is written under the schema's @@ -343,6 +347,7 @@ class the schema declares that a box can be written as; an answer naming one "model_ref": outcome.model_ref, "assets_skipped": outcome.assets_skipped, "regions_discarded": outcome.regions_discarded, + "regions_out_of_bounds": outcome.regions_out_of_bounds, } diff --git a/src/visionset/server/errors.py b/src/visionset/server/errors.py index f0c4c44a..07b0af81 100644 --- a/src/visionset/server/errors.py +++ b/src/visionset/server/errors.py @@ -44,6 +44,7 @@ from starlette.responses import JSONResponse, Response from visionset.kernel import ( + AnnotationGeometryOutOfBounds, AnnotationNotFound, AnnotationNotFromModel, AssetNotFound, @@ -334,6 +335,7 @@ class ErrorRule: InvalidAnnotation: ErrorRule(422, "INVALID_ANNOTATION"), LabelClassNotInSchema: ErrorRule(422, "LABEL_CLASS_NOT_IN_SCHEMA"), DisallowedGeometry: ErrorRule(422, "DISALLOWED_GEOMETRY"), + AnnotationGeometryOutOfBounds: ErrorRule(422, "ANNOTATION_GEOMETRY_OUT_OF_BOUNDS"), # 422 like its five siblings, not 409, and the split is the one this table is # built on. A 409 says "the resource's state refuses this; change the state # and resubmit" — but the state to change is the annotation set, and removing diff --git a/src/visionset/server/models.py b/src/visionset/server/models.py index 96118f98..40a77dae 100644 --- a/src/visionset/server/models.py +++ b/src/visionset/server/models.py @@ -933,8 +933,9 @@ class PreLabelRunOut(BaseModel): set before the first forward pass, so both its progress and its total are counted in the unit its own work is over. - **The outcome, once the job has one.** `stopped_early`, `assets_labeled` and - `regions_discarded` are the handler's own account of what a settled run did. + **The outcome, once the job has one.** `stopped_early`, `assets_labeled`, + `regions_discarded` and `regions_out_of_bounds` are the handler's own + account of what a settled run did. They are `null` while the job is still `queued` or `running`, and `null` where it ended `failed` before producing one — but a `cancelled` run still carries them: stopping partway is a coherent outcome for a handler whose @@ -962,6 +963,10 @@ class PreLabelRunOut(BaseModel): #: for, discarded rather than written. `null` until the job settles with a #: result. regions_discarded: int | None + #: Regions whose mapped geometry had no overlap with a measured asset, + #: discarded rather than written. `null` until the job settles with a + #: result. + regions_out_of_bounds: int | None @classmethod def of(cls, run: PreLabelRun) -> Self: @@ -974,6 +979,7 @@ def of(cls, run: PreLabelRun) -> Self: stopped_early=run.stopped_early, assets_labeled=run.assets_labeled, regions_discarded=run.regions_discarded, + regions_out_of_bounds=run.regions_out_of_bounds, ) diff --git a/src/visionset/wire/__init__.py b/src/visionset/wire/__init__.py index fca2b946..1b6d9345 100644 --- a/src/visionset/wire/__init__.py +++ b/src/visionset/wire/__init__.py @@ -440,8 +440,9 @@ def pre_label_run(value: PreLabelRun) -> dict[str, Any]: Assets, on ``weight_download``'s and ``integrity_check``'s terms: the handler's own unit, named where its job type is known. ``stopped_early``, - ``assets_labeled`` and ``regions_discarded`` are null until the job settles - with a result — a cancelled run still carries them, a failed one never does. + ``assets_labeled``, ``regions_discarded`` and ``regions_out_of_bounds`` are + null until the job settles with a result — a cancelled run still carries + them, a failed one never does. """ return { "job_id": str(value.job_id), @@ -452,6 +453,7 @@ def pre_label_run(value: PreLabelRun) -> dict[str, Any]: "stopped_early": value.stopped_early, "assets_labeled": value.assets_labeled, "regions_discarded": value.regions_discarded, + "regions_out_of_bounds": value.regions_out_of_bounds, } diff --git a/tests/cli/test_batch_commands.py b/tests/cli/test_batch_commands.py index f3e69514..c0f3cfe3 100644 --- a/tests/cli/test_batch_commands.py +++ b/tests/cli/test_batch_commands.py @@ -267,6 +267,7 @@ def test_pre_label_json_emits_the_complete_outcome( "stopped_early": False, "assets_skipped": 0, "regions_discarded": 0, + "regions_out_of_bounds": 0, } diff --git a/tests/cli/test_json_contract.py b/tests/cli/test_json_contract.py index db7d2eec..1569234f 100644 --- a/tests/cli/test_json_contract.py +++ b/tests/cli/test_json_contract.py @@ -24,6 +24,7 @@ import json from datetime import UTC, datetime from typing import Any +from uuid import uuid4 import pytest from pydantic import BaseModel @@ -59,7 +60,7 @@ from visionset import wire from visionset.formats._dummy import DummyExporter -from visionset.kernel.domain import AssetProgress +from visionset.kernel.domain import AssetProgress, BackgroundJobState, PreLabelRun from visionset.server import models # One row per pair: a label, the projected payload, and the wire model it must @@ -211,6 +212,26 @@ def test_an_empty_listing_is_still_an_object() -> None: assert wire.page([]) == {"items": [], "total": 0} +def test_a_settled_pre_label_run_has_wire_parity_when_nested_in_a_batch() -> None: + run = PreLabelRun( + batch_id=BATCH.id, + job_id=uuid4(), + state=BackgroundJobState.SUCCEEDED, + assets_processed=2, + assets_total=2, + stopped_early=False, + assets_labeled=1, + regions_discarded=2, + regions_out_of_bounds=3, + ) + + projected = wire.batch(BATCH, COUNTS, promoted=frozenset(), pre_labeled=run) + + assert set(projected["pre_label_run"] or {}) == set(models.PreLabelRunOut.model_fields) + models.BatchOut.model_validate(projected) + json.dumps(projected) + + # --- the timestamp format the parity gate depends on ------------------------- diff --git a/tests/inference/test_prelabel.py b/tests/inference/test_prelabel.py index 159427e2..19185a17 100644 --- a/tests/inference/test_prelabel.py +++ b/tests/inference/test_prelabel.py @@ -177,6 +177,8 @@ def __init__( classes: tuple[LabelClass, ...] = (SIGN, POST, LANE), pool_kind: str = "detector", regions: tuple[PredictedRegion, ...] = DEFAULT_REGIONS, + asset_count: int = 3, + asset_size: tuple[int, int] | None = None, ) -> None: self.workspace = WorkspaceService.init(tmp_path / name) self.batches = BatchService(self.workspace) @@ -186,7 +188,14 @@ def __init__( self.connections = InferenceConnectionService(self.workspace) self.project = ProjectService(self.workspace).create(f"{name}-project") self.schemas.create_version(self.project.id, list(classes)) - self.assets = [self._asset(f"{name}-{index}") for index in range(3)] + self.assets = [ + self._asset( + f"{name}-{index}", + width=None if asset_size is None else asset_size[0], + height=None if asset_size is None else asset_size[1], + ) + for index in range(asset_count) + ] self.batch = self.batches.create(self.project.id, "first", self.assets) self.batches.approve(self.batch.id) self._job_id = self.batches.jobs(self.batch.id)[0].id @@ -201,7 +210,7 @@ def __init__( ) self.pool = FakeProviderPool(kind=pool_kind, regions=regions) - def _asset(self, seed: str) -> UUID: + def _asset(self, seed: str, *, width: int | None = None, height: int | None = None) -> UUID: content_hash = self.workspace.blob_store.put(BytesIO(seed.encode())) with self.workspace.unit_of_work() as uow: return uow.assets.add( @@ -209,6 +218,8 @@ def _asset(self, seed: str) -> UUID: project_id=self.project.id, content_hash=content_hash, uri=f"/tmp/{seed}.png", + width=width, + height=height, ) ).id @@ -288,6 +299,65 @@ def only_unmappable_fixture(tmp_path: Path) -> Iterator[Fixture]: fixture.close() +@pytest.fixture +def partly_off_frame_fixture(tmp_path: Path) -> Iterator[Fixture]: + fixture = Fixture( + tmp_path, + "partly-off-frame", + asset_count=1, + asset_size=(100, 80), + regions=( + *DEFAULT_REGIONS, + PredictedRegion( + label="post", + confidence=0.9, + geometry=BboxGeometry(x=101.0, y=10.0, width=5.0, height=5.0), + ), + ), + ) + yield fixture + fixture.close() + + +@pytest.fixture +def only_off_frame_fixture(tmp_path: Path) -> Iterator[Fixture]: + fixture = Fixture( + tmp_path, + "only-off-frame", + asset_count=1, + asset_size=(100, 80), + regions=( + PredictedRegion( + label="post", + confidence=0.9, + geometry=BboxGeometry(x=101.0, y=10.0, width=5.0, height=5.0), + ), + ), + ) + yield fixture + fixture.close() + + +@pytest.fixture +def early_stop_off_frame_fixture(tmp_path: Path) -> Iterator[Fixture]: + fixture = Fixture( + tmp_path, + "early-stop-off-frame", + asset_count=2, + asset_size=(100, 80), + regions=( + *DEFAULT_REGIONS, + PredictedRegion( + label="post", + confidence=0.9, + geometry=BboxGeometry(x=101.0, y=10.0, width=5.0, height=5.0), + ), + ), + ) + yield fixture + fixture.close() + + @pytest.fixture def capitalized_class_fixture(tmp_path: Path) -> Iterator[Fixture]: """A schema class spelled with a capital, against the casefolded answer a @@ -651,3 +721,64 @@ def test_an_asset_whose_only_regions_are_unmappable_stays_unannotated( for asset_id in only_unmappable_fixture.assets: assert job.progress[asset_id] is AssetProgress.UNANNOTATED assert only_unmappable_fixture.annotations_on(asset_id) == [] + + +def test_a_region_wholly_outside_a_measured_asset_is_discarded( + partly_off_frame_fixture: Fixture, +) -> None: + outcome = pre_label( + partly_off_frame_fixture.workspace, + batch_id=partly_off_frame_fixture.batch.id, + connection_id=partly_off_frame_fixture.connection.id, + pool=partly_off_frame_fixture.pool, + ) + + assert outcome.annotations_written == 1 + assert outcome.regions_discarded == 0 + assert outcome.regions_out_of_bounds == 1 + assert [ + annotation.geometry + for annotation in partly_off_frame_fixture.annotations_on( + partly_off_frame_fixture.assets[0] + ) + ] == [BboxGeometry(x=1.0, y=2.0, width=3.0, height=4.0)] + + +def test_an_asset_whose_only_regions_are_off_frame_stays_unannotated( + only_off_frame_fixture: Fixture, +) -> None: + outcome = pre_label( + only_off_frame_fixture.workspace, + batch_id=only_off_frame_fixture.batch.id, + connection_id=only_off_frame_fixture.connection.id, + pool=only_off_frame_fixture.pool, + ) + + assert outcome.assets_labeled == 0 + assert outcome.regions_discarded == 0 + assert outcome.regions_out_of_bounds == 1 + job = only_off_frame_fixture.job() + assert job.progress[only_off_frame_fixture.assets[0]] is AssetProgress.UNANNOTATED + assert only_off_frame_fixture.annotations_on(only_off_frame_fixture.assets[0]) == [] + + +def test_stopping_after_an_off_frame_region_keeps_its_count( + early_stop_off_frame_fixture: Fixture, +) -> None: + seen = iter([False, True]) + + outcome = pre_label( + early_stop_off_frame_fixture.workspace, + batch_id=early_stop_off_frame_fixture.batch.id, + connection_id=early_stop_off_frame_fixture.connection.id, + should_stop=lambda: next(seen), + pool=early_stop_off_frame_fixture.pool, + ) + + assert outcome.stopped_early is True + assert outcome.annotations_written == 1 + assert outcome.regions_discarded == 0 + assert outcome.regions_out_of_bounds == 1 + job = early_stop_off_frame_fixture.job() + assert job.progress[early_stop_off_frame_fixture.assets[0]] is AssetProgress.PRE_LABELED + assert job.progress[early_stop_off_frame_fixture.assets[1]] is AssetProgress.UNANNOTATED diff --git a/tests/jobs/test_prelabel_handler.py b/tests/jobs/test_prelabel_handler.py index 5f772ca0..70868512 100644 --- a/tests/jobs/test_prelabel_handler.py +++ b/tests/jobs/test_prelabel_handler.py @@ -134,6 +134,7 @@ def fake_pre_label( "stopped_early": False, "assets_skipped": 0, "regions_discarded": 0, + "regions_out_of_bounds": 0, } assert captured["batch_id"] == batch_id assert captured["connection_id"] == connection_id diff --git a/tests/kernel/test_annotation_service.py b/tests/kernel/test_annotation_service.py index eda30778..a0aa6ac4 100644 --- a/tests/kernel/test_annotation_service.py +++ b/tests/kernel/test_annotation_service.py @@ -17,6 +17,7 @@ from sqlalchemy import text from visionset.kernel import ( + AnnotationGeometryOutOfBounds, AnnotationNotFound, AnnotationNotFromModel, AssetNotInJob, @@ -129,6 +130,8 @@ def __init__( *, assets: int = 3, classes: Sequence[LabelClass] = (SIGN, LANE, KIOSK), + asset_width: int | None = None, + asset_height: int | None = None, ) -> None: self.workspace = WorkspaceService.init(tmp_path / name) self.batches = BatchService(self.workspace) @@ -137,6 +140,8 @@ def __init__( self.annotations = AnnotationService(self.workspace) self.project = ProjectService(self.workspace).create(f"{name}-project") self.schemas.create_version(self.project.id, list(classes)) + self.asset_width = asset_width + self.asset_height = asset_height self.assets = [self._asset(f"{name}-{index}") for index in range(assets)] self.batch = self.batches.create(self.project.id, "first", self.assets) @@ -148,6 +153,8 @@ def _asset(self, seed: str) -> UUID: project_id=self.project.id, content_hash=content_hash, uri=f"/tmp/{seed}.png", + width=self.asset_width, + height=self.asset_height, ) ).id @@ -325,6 +332,107 @@ def test_the_geometry_rule_is_per_class_not_the_versions_union(tmp_path: Path) - fixture.close() +def test_add_refuses_a_box_wholly_outside_a_measured_asset(tmp_path: Path) -> None: + fixture = Fixture(tmp_path, asset_width=100, asset_height=80) + job = fixture.working() + + with pytest.raises(AnnotationGeometryOutOfBounds) as raised: + fixture.annotations.add( + job.id, + [_box(fixture.assets[0], geometry=BboxGeometry(x=101, y=20, width=5, height=5))], + ) + + assert raised.value.index == 0 + assert fixture.annotations.for_asset(job.id, fixture.assets[0]) == [] + assert fixture.progress_of(job, fixture.assets[0]) is UNANNOTATED + fixture.close() + + +def test_add_accepts_a_box_partially_overlapping_a_measured_asset(tmp_path: Path) -> None: + fixture = Fixture(tmp_path, asset_width=100, asset_height=80) + job = fixture.working() + + stored = fixture.annotations.add( + job.id, + [_box(fixture.assets[0], geometry=BboxGeometry(x=99, y=20, width=5, height=5))], + ) + + assert len(stored) == 1 + fixture.close() + + +def test_add_accepts_a_box_outside_an_unmeasured_asset(tmp_path: Path) -> None: + fixture = Fixture(tmp_path) + job = fixture.working() + + stored = fixture.annotations.add( + job.id, + [_box(fixture.assets[0], geometry=BboxGeometry(x=101, y=20, width=5, height=5))], + ) + + assert len(stored) == 1 + fixture.close() + + +def test_add_rolls_back_when_its_second_box_is_outside_a_measured_asset(tmp_path: Path) -> None: + fixture = Fixture(tmp_path, asset_width=100, asset_height=80) + job = fixture.working() + + with pytest.raises(AnnotationGeometryOutOfBounds) as raised: + fixture.annotations.add( + job.id, + [ + _box(fixture.assets[0]), + _box(fixture.assets[1], geometry=BboxGeometry(x=101, y=20, width=5, height=5)), + ], + ) + + assert raised.value.index == 1 + for asset_id in fixture.assets[:2]: + assert fixture.annotations.for_asset(job.id, asset_id) == [] + assert fixture.progress_of(job, asset_id) is UNANNOTATED + fixture.close() + + +def test_update_refuses_replacement_geometry_outside_the_stored_asset(tmp_path: Path) -> None: + fixture = Fixture(tmp_path, asset_width=100, asset_height=80) + job = fixture.working() + (stored,) = fixture.annotations.add(job.id, [_box(fixture.assets[0])]) + + with pytest.raises(AnnotationGeometryOutOfBounds) as raised: + fixture.annotations.update( + job.id, + [stored.model_copy(update={"geometry": BboxGeometry(x=101, y=20, width=5, height=5)})], + ) + + assert raised.value.index == 0 + assert fixture.annotations.get(stored.id) == stored + fixture.close() + + +def test_enter_unreviewed_refuses_a_model_box_outside_a_measured_asset(tmp_path: Path) -> None: + fixture = Fixture(tmp_path, asset_width=100, asset_height=80) + job = fixture.working() + + with pytest.raises(AnnotationGeometryOutOfBounds) as raised: + fixture.annotations.enter_unreviewed( + job.id, + [ + _box( + fixture.assets[0], + geometry=BboxGeometry(x=101, y=20, width=5, height=5), + provenance="model", + model_ref="model-v1", + ) + ], + ) + + assert raised.value.index == 0 + assert fixture.annotations.for_asset(job.id, fixture.assets[0]) == [] + assert fixture.progress_of(job, fixture.assets[0]) is UNANNOTATED + fixture.close() + + #: One class, two shapes. The whole point of #584: a sign photographed close up is #: worth outlining and one at the end of the street is worth boxing, and they are #: the same class. diff --git a/tests/kernel/test_geometry.py b/tests/kernel/test_geometry.py index 311a03f4..20dda60b 100644 --- a/tests/kernel/test_geometry.py +++ b/tests/kernel/test_geometry.py @@ -14,6 +14,7 @@ PolygonGeometry, PolylineGeometry, ) +from visionset.kernel.domain.geometry import geometry_intersects_asset geometry_adapter: TypeAdapter[Geometry] = TypeAdapter(Geometry) @@ -208,3 +209,38 @@ def test_implemented_geometries_names_exactly_the_variants_of_the_union() -> Non } assert IMPLEMENTED_GEOMETRIES == from_the_union == expected assert set(GeometryType) > IMPLEMENTED_GEOMETRIES # the rest is roadmap + + +def test_geometry_intersects_asset_skips_an_unmeasured_frame() -> None: + box = BboxGeometry(x=200.0, y=100.0, width=10.0, height=10.0) + assert geometry_intersects_asset(box, width=None, height=80) + assert geometry_intersects_asset(box, width=100, height=None) + + +@pytest.mark.parametrize( + ("geometry", "expected"), + [ + (BboxGeometry(x=-10, y=10, width=20, height=20), True), + (BboxGeometry(x=100, y=10, width=5, height=20), True), + (BboxGeometry(x=101, y=10, width=5, height=20), False), + (PolylineGeometry(points=[(-10, 40), (110, 40)]), True), + (PolylineGeometry(points=[(-10, -10), (-5, -5)]), False), + (PolygonGeometry(points=[(-10, -10), (110, -10), (110, 90), (-10, 90)]), True), + (PolygonGeometry(points=[(110, 90), (120, 90), (120, 100)]), False), + (ClassificationGeometry(), True), + ], +) +def test_geometry_intersects_asset_handles_every_supported_variant( + geometry: Geometry, expected: bool +) -> None: + assert geometry_intersects_asset(geometry, width=100, height=80) is expected + + +def test_geometry_intersects_asset_accepts_a_polygon_that_encloses_the_frame() -> None: + polygon = PolygonGeometry(points=[(-10, -10), (110, -10), (110, 90), (-10, 90)]) + assert geometry_intersects_asset(polygon, width=100, height=80) + + +def test_geometry_intersects_asset_accepts_non_finite_coordinates() -> None: + box = BboxGeometry(x=float("nan"), y=100.0, width=10.0, height=10.0) + assert geometry_intersects_asset(box, width=100, height=80) diff --git a/tests/kernel/test_pre_label_runs.py b/tests/kernel/test_pre_label_runs.py index 70da91f1..d8fa8f2d 100644 --- a/tests/kernel/test_pre_label_runs.py +++ b/tests/kernel/test_pre_label_runs.py @@ -131,6 +131,7 @@ def test_the_outcome_is_null_before_the_job_settles() -> None: assert run.stopped_early is None assert run.assets_labeled is None assert run.regions_discarded is None + assert run.regions_out_of_bounds is None def test_a_succeeded_run_carries_the_handlers_outcome() -> None: @@ -148,6 +149,7 @@ def test_a_succeeded_run_carries_the_handlers_outcome() -> None: "stopped_early": False, "assets_skipped": 1, "regions_discarded": 2, + "regions_out_of_bounds": 1, }, ) @@ -156,6 +158,26 @@ def test_a_succeeded_run_carries_the_handlers_outcome() -> None: assert run.stopped_early is False assert run.assets_labeled == 6 assert run.regions_discarded == 2 + assert run.regions_out_of_bounds == 1 + + +def test_a_pre_label_result_ignores_boolean_and_malformed_counts() -> None: + job = BackgroundJob( + type=PRE_LABEL_JOB_TYPE, + payload=pre_label_job_payload(uuid4(), uuid4(), 0.35), + state=BackgroundJobState.SUCCEEDED, + result={ + "assets_labeled": True, + "regions_discarded": "two", + "regions_out_of_bounds": 1.0, + }, + ) + + run = PreLabelRun.of(job) + + assert run.assets_labeled is None + assert run.regions_discarded is None + assert run.regions_out_of_bounds is None def test_a_failed_run_keeps_the_sentence_and_has_no_outcome() -> None: @@ -184,7 +206,12 @@ def test_a_cancelled_run_still_carries_its_outcome() -> None: state=BackgroundJobState.CANCELLED, processed=12, total=48, - result={"stopped_early": True, "assets_labeled": 4, "regions_discarded": 0}, + result={ + "stopped_early": True, + "assets_labeled": 4, + "regions_discarded": 0, + "regions_out_of_bounds": 0, + }, ) run = PreLabelRun.of(job) @@ -227,7 +254,12 @@ def test_a_settled_run_stays_readable(workspace: WorkspaceService, batches: Batc job.id, BackgroundJobOutcome( state=BackgroundJobState.SUCCEEDED, - result={"stopped_early": False, "assets_labeled": 3, "regions_discarded": 1}, + result={ + "stopped_early": False, + "assets_labeled": 3, + "regions_discarded": 1, + "regions_out_of_bounds": 2, + }, processed=5, total=5, ), @@ -239,6 +271,7 @@ def test_a_settled_run_stays_readable(workspace: WorkspaceService, batches: Batc assert run.state is BackgroundJobState.SUCCEEDED assert run.assets_labeled == 3 assert run.regions_discarded == 1 + assert run.regions_out_of_bounds == 2 def test_a_cancelled_run_reports_how_far_it_got( @@ -251,7 +284,12 @@ def test_a_cancelled_run_reports_how_far_it_got( job.id, BackgroundJobOutcome( state=BackgroundJobState.CANCELLED, - result={"stopped_early": True, "assets_labeled": 4, "regions_discarded": 0}, + result={ + "stopped_early": True, + "assets_labeled": 4, + "regions_discarded": 0, + "regions_out_of_bounds": 0, + }, processed=12, total=48, ), diff --git a/tests/mcp/test_batch_tools.py b/tests/mcp/test_batch_tools.py index 7cebbb37..a7e56806 100644 --- a/tests/mcp/test_batch_tools.py +++ b/tests/mcp/test_batch_tools.py @@ -514,13 +514,15 @@ def test_pre_labeling_blocks_and_returns_what_it_wrote( outcome = payload(call("pre_label_batch", batch_id=batch_id, connection=connection_id)) - assert outcome["assets_considered"] == 2 - assert outcome["assets_labeled"] == 2 - assert outcome["annotations_written"] == 2 - assert outcome["model_ref"] == "acme/detector@abc123" - assert outcome["assets_skipped"] == 0 - assert outcome["regions_discarded"] == 0 - assert "stopped_early" not in outcome + assert outcome == { + "assets_considered": 2, + "assets_labeled": 2, + "annotations_written": 2, + "model_ref": "acme/detector@abc123", + "assets_skipped": 0, + "regions_discarded": 0, + "regions_out_of_bounds": 0, + } assert payload(call("get_batch", batch_id=batch_id))["progress"]["pre_labeled"] == 2 diff --git a/tests/server/test_annotations.py b/tests/server/test_annotations.py index 6aeb3cf8..94d496c0 100644 --- a/tests/server/test_annotations.py +++ b/tests/server/test_annotations.py @@ -108,6 +108,44 @@ def test_one_call_can_carry_labels_for_several_assets( assert client.get(f"/jobs/{job_id}/progress").json()["annotated"] == 2 +def test_a_box_wholly_outside_a_measured_asset_is_a_422( + client: TestClient, working: tuple[str, str], assets: list[str] +) -> None: + _, job_id = working + + response = client.post( + f"/jobs/{job_id}/annotations", + json=[ + a_box( + assets[0], + geometry={"type": "bbox", "x": 33.0, "y": 12.0, "width": 5.0, "height": 5.0}, + ) + ], + ) + + assert response.status_code == 422 + assert response.json()["code"] == "ANNOTATION_GEOMETRY_OUT_OF_BOUNDS" + + +def test_a_box_partly_overlapping_a_measured_asset_is_stored( + client: TestClient, working: tuple[str, str], assets: list[str] +) -> None: + _, job_id = working + + response = client.post( + f"/jobs/{job_id}/annotations", + json=[ + a_box( + assets[0], + geometry={"type": "bbox", "x": 31.0, "y": 12.0, "width": 5.0, "height": 5.0}, + ) + ], + ) + + assert response.status_code == 201 + assert response.json()["total"] == 1 + + def test_a_polygon_lands_under_the_class_that_declares_one( client: TestClient, working: tuple[str, str], assets: list[str] ) -> None: diff --git a/tests/server/test_errors.py b/tests/server/test_errors.py index 83fc724e..32bb4f59 100644 --- a/tests/server/test_errors.py +++ b/tests/server/test_errors.py @@ -109,6 +109,7 @@ "InvalidAnnotation": (422, "INVALID_ANNOTATION"), "LabelClassNotInSchema": (422, "LABEL_CLASS_NOT_IN_SCHEMA"), "DisallowedGeometry": (422, "DISALLOWED_GEOMETRY"), + "AnnotationGeometryOutOfBounds": (422, "ANNOTATION_GEOMETRY_OUT_OF_BOUNDS"), "DuplicateClassificationTag": (422, "DUPLICATE_CLASSIFICATION_TAG"), "MissingRequiredAttribute": (422, "MISSING_REQUIRED_ATTRIBUTE"), "UnknownAttribute": (422, "UNKNOWN_ATTRIBUTE"),