diff --git a/CHANGELOG.md b/CHANGELOG.md index efe5c79e..1e790619 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,9 @@ nothing was being distributed. This is the first version that is. `rot90`) and a number of variants per train-fold image - served as `/projects/{id}/preprocessing-recipes` with a preview at `POST /projects/{id}/preprocessing-preview`, `visionset recipe`, and the - `create_preprocessing_recipe`, `list_preprocessing_recipes` and (behind `--allow-destructive`) - `delete_preprocessing_recipe` tools. An export names one beside its target - `recipe=` on the + `create_preprocessing_recipe`, `list_preprocessing_recipes`, `get_preprocessing_recipe`, + `update_preprocessing_recipe` and (behind `--allow-destructive`) `delete_preprocessing_recipe` + tools. An export names one beside its target - `recipe=` on the export and compatibility routes, `visionset export --recipe`, `recipe` on `export_release` and `check_export` - and keeps the spec by value: the job carries a snapshot, and the export report gains a `preprocessing` block with the spec, its hash, the Pillow version and a mapping from diff --git a/docs/content/architecture/backend/README.md b/docs/content/architecture/backend/README.md index 5af9949e..cefb6b8a 100644 --- a/docs/content/architecture/backend/README.md +++ b/docs/content/architecture/backend/README.md @@ -58,7 +58,7 @@ upward, and the three surfaces do not point at each other. | [`cli`](../../../../src/visionset/cli/) | Typer. The whole cycle from a shell. | [cli.md](cli.md) | | [`mcp`](../../../../src/visionset/mcp/) | The MCP tool surface, for agents. | [mcp.md](mcp.md) | | [`formats`](../../../../src/visionset/formats/) | Exporter plugins, discovered by entry point. | [formats.md](formats.md) | -| [`preprocessing`](../../../../src/visionset/preprocessing/) | Pre-processing drivers - the Pillow resize and augmentation engines behind the `PreprocessingDriver` port - discovered over the `visionset.preprocessing` entry-point group the way exporters are. | [formats.md](formats.md#the-sibling-group-preprocessing-drivers) | +| [`preprocessing`](../../../../src/visionset/preprocessing/) | Pre-processing drivers - the Pillow resize and augmentation engines behind the `PreprocessingDriver` port - discovered over the `visionset.preprocessing` entry-point group the way exporters are. | [preprocessing-drivers.md](preprocessing-drivers.md) | | [`wire`](../../../../src/visionset/wire/) | The JSON shapes the CLI and MCP publish. | [wire.md](wire.md) | | [`jobs`](../../../../src/visionset/jobs/) | Handlers for work that outlives a request. | [jobs.md](jobs.md) | | [`inference`](../../../../src/visionset/inference/) | Where a model connection becomes a running model, and which model families could run next. | [inference.md](inference.md) | diff --git a/docs/content/architecture/backend/formats.md b/docs/content/architecture/backend/formats.md index dd546edb..1a04e25b 100644 --- a/docs/content/architecture/backend/formats.md +++ b/docs/content/architecture/backend/formats.md @@ -86,7 +86,9 @@ shape. The two built-in drivers, `pillow-resize` and `pillow-augment`, live in [`pillow/`](../../../../src/visionset/preprocessing/pillow/). The kernel takes driver instances through `ReleaseService.export(..., drivers=)` and never a name, and the purity contract forbids it importing this package for the reason it forbids -`formats`. [`docs/content/preprocessing.md`](../../preprocessing.md) covers what a +`formats`. [`preprocessing-drivers.md`](preprocessing-drivers.md) is the guide to +writing one - the port, the entry-point group, the dependency rule and the admission +tests - and [`docs/content/preprocessing.md`](../../preprocessing.md) covers what a recipe is and what the drivers promise. A plugin also gets a `ContentReader` and never a `BlobStore`: a reader can read diff --git a/docs/content/architecture/backend/preprocessing-drivers.md b/docs/content/architecture/backend/preprocessing-drivers.md new file mode 100644 index 00000000..43619a54 --- /dev/null +++ b/docs/content/architecture/backend/preprocessing-drivers.md @@ -0,0 +1,194 @@ +# Writing a pre-processing driver + +**This is not a public extension API in this release.** The contract described here exists so +that this distribution's own drivers run on it, and it may change in any release without notice +or a migration path. Nothing here is stable, and nothing here is supported for out-of-tree use +yet. It is written down because somebody working inside this repository, or experimenting +alongside it, needs to be able to read what the contract actually is. + +**Installing a driver is trusting its author with code execution in the workers**, exactly as +installing any `pip` package is. Discovery loads the class an entry point names, in the server +process and in every background worker, and calls it. There is no sandbox at this layer. + +## What a driver is, and what it is not + +A [recipe](../../preprocessing.md) is a value: at most one resize step followed by augmentation +steps, and a number of variants per train-fold image. The kernel owns everything about a recipe +except the pixels. It validates the grammar, hashes the spec, decides which files an export +writes, derives every variant's seed, and moves every annotation +([`kernel/domain/preprocessing_transform.py`](../../../../src/visionset/kernel/domain/preprocessing_transform.py)). +A driver is the pixel engine for one kind of step: it is handed one image's bytes and one step +and answers with the transformed bytes. + +**Drivers do pixels only.** A driver that moved annotations would be a second spelling of +arithmetic the kernel already owns, and the two spellings would drift. What keeps a variant's +labels on its pixels is that both sides read the same arithmetic: `letterbox_fit` is the one +statement of where letterboxed content lands, and the per-variant draws come from the kernel's +seed helpers, never from a driver's own random source. + +## The port + +[`PreprocessingDriver`](../../../../src/visionset/kernel/ports/preprocessing.py) is a +`@runtime_checkable` protocol with one data member and one method, so callers ask with +`isinstance` on an *instance*: + +| Member | What it is | +| --- | --- | +| `step_kinds: frozenset[str]` | Which step kinds this driver applies - `{"resize"}`, `{"augment"}`, or both | +| `apply(step, image, *, seed, variant) -> bytes` | One image's bytes through one step, in the source's encoding | + +`step` is a `ResizeStep` or an `AugmentStep`, the kernel's own models, and a driver reads the +step's fields for what to do: `strategy`, `width`, `height` and `pad_value` for a resize; +`op` and `amount` for an augmentation. `image` is the source bytes as the content store holds +them. `seed` is the variant's digest from `variant_seed` and `variant` its index; variant 0 is +the base image, so a step applied to it returns the bytes untouched apart from orientation and +re-encoding, and a resize reads neither argument because it is deterministic. + +**Every draw comes from the kernel's seed helpers.** The two that draw are +`brightness_contrast_factors(seed, amount)` and `rot90_quarter_turns(seed)`, both exported from +`visionset.kernel.domain`; `hflip` draws nothing and always mirrors. A driver that read the seed +any other way, or drew from anywhere else, would put its pixels where the geometry transform did +not put the labels. The built-in `rot90` turns counter-clockwise because the kernel's +`_rotated_once` does, and a driver applying that op must turn the same way. + +The step grammar is closed. Which kinds exist, and which augmentation ops, is decided by the +kernel's `Step` union, so a driver applies one of the kinds the kernel already names and cannot +introduce a `crop`; a driver claiming a kind the built-in pair applies replaces it for that kind. + +## Instances, never names + +The kernel may not import this package - the purity contract forbids `visionset.preprocessing` +for the reason it forbids `visionset.formats` - so it never scans entry points and never resolves +a name. `ReleaseService.export(..., drivers=)` and the preview take a mapping from step kind to +driver *instance*, and whoever composed the call built that mapping. +[`preprocessing/registry.py`](../../../../src/visionset/preprocessing/registry.py) is where the +surfaces build it: `drivers()` scans the group and keys what it finds by every step kind the +driver declares, `pick()` and `driver_for()` resolve one step, and a kind nothing applies is +refused with `PreprocessingDriverNotFound` naming what is installed. Nothing is cached; a driver +installed while a server is running is seen on the next call. + +## Registering it + +A driver is discovered through the `visionset.preprocessing` entry-point group, as an exporter is +through `visionset.formats`. The entry point names a **class**, which discovery calls with no +arguments: + +```toml +[project.entry-points."visionset.preprocessing"] +my-resize = "my_package.driver:MyResizeDriver" +``` + +The two built-in drivers register the same way, in this repository's own `pyproject.toml`: + +```toml +[project.entry-points."visionset.preprocessing"] +pillow-resize = "visionset.preprocessing.pillow:PillowResizeDriver" +pillow-augment = "visionset.preprocessing.pillow:PillowAugmentDriver" +``` + +`step_kinds` is read straight after that call, so it must be a class attribute or set in a +no-argument constructor, and it must be answerable without the driver's image library loaded. + +**The image library is the driver's own.** Pillow is a dependency of this distribution for the +built-in pair in [`preprocessing/pillow/`](../../../../src/visionset/preprocessing/pillow/) and +is used nowhere else; a driver from outside the distribution brings whatever it decodes and +encodes with, declared in its own `dependencies`, and imports it inside the module that needs it +rather than at the entry point's import path. Nothing in the kernel names an image library, which +is what lets a driver use one the kernel has never heard of. + +## What a driver promises about bytes + +The built-in pair's promise is the one an export report is written against: a JPEG comes back a +JPEG at quality 95 with its chroma subsampling kept, a PNG a lossless PNG, and any other encoding +a PNG; no metadata travels. A driver of your own sets its own encoding policy, and the reader of +its output learns it from the bytes, so say what it is in the driver's docstring. + +Byte stability is promised within one environment only. The report records `pillow_version` +because the pixels a resize or an enhancement produces depend on the codec and resampling code +that produced them; a driver that used a different library would make that field describe the +wrong thing, which is a limit of the report's shape in this release rather than of the driver. + +## The smallest driver that works + +```python +from typing import Final + +from visionset.kernel.domain import ResizeStep, ResizeStrategy, Step, letterbox_fit + + +class MyResizeDriver: + """Satisfies `PreprocessingDriver` structurally.""" + + step_kinds: Final = frozenset({"resize"}) + + def apply(self, step: Step, image: bytes, *, seed: bytes, variant: int) -> bytes: + if not isinstance(step, ResizeStep): + raise TypeError(f"{type(self).__name__} applies resize steps, not {step.kind!r}") + import my_image_library # the driver's own, loaded on first use + + source = my_image_library.decode(image) + if step.strategy is ResizeStrategy.STRETCH: + result = source.resize(step.width, step.height) + else: + fit = letterbox_fit( + source.width, source.height, target_width=step.width, target_height=step.height + ) + content = source.resize(fit.content_width, fit.content_height) + result = my_image_library.canvas(step.width, step.height, grey=step.pad_value) + result.paste(content, at=(fit.offset_x, fit.offset_y)) + return my_image_library.encode(result, like=source) +``` + +The letterbox arithmetic is the kernel's, read back through `letterbox_fit` rather than +re-derived, so the content lands exactly where the geometry transform placed the annotations - a +letterbox worked out in the driver would be a second spelling that could be half a pixel off. An +augmentation driver reads the seed the same way: `brightness_contrast_factors(seed, step.amount)` +and `rot90_quarter_turns(seed)`, and mirrors on every `hflip` variant. + +Every refusal a driver raises derives from `VisionSetError` in +[`kernel/errors.py`](../../../../src/visionset/kernel/errors.py); bytes that do not decode are +`UnsupportedMedia`. An implementation library's exception reaching a surface is a stack trace +where a sentence belongs. + +## Admission + +[`tests/preprocessing/test_driver_registry.py`](../../../../tests/preprocessing/test_driver_registry.py) +is what discovery holds a driver to, and what a driver of your own has to pass alongside the +built-in pair: + +- The class the entry point names, called with no arguments, satisfies `PreprocessingDriver` + under `isinstance`. A class without `step_kinds` is dropped by the port and never keyed, with + no error - the same silence a format plugin that fails the port gets. +- Every step kind in `step_kinds` is keyed to the driver that declares it, and a driver declaring + two kinds appears under both. +- A kind nothing installed applies is refused with `PreprocessingDriverNotFound`, whose message + and `installed` attribute name the kinds that are, or `none`. +- The suite asserts the built-in pair is what answers `resize` and `augment`, so a driver that + replaces one of them fails that assertion by design; it is the record of which driver the + distribution ships for each kind, and a replacement changes that record. + +Run it against your own driver by installing the driver, not by injecting a fake. From a checkout +of this repository: + +```bash +uv pip install --python .venv -e /path/to/your-driver +uv run --no-sync python -c "from visionset.preprocessing.registry import drivers; print(sorted(drivers()))" +uv run --no-sync pytest tests/preprocessing -v +``` + +The middle line is not optional: entry-point metadata is recorded at install time, and if your +driver's kinds are absent from that list the suite exercises the built-in pair and nothing you +wrote. After any pull that changes `[project.entry-points]`, the metadata in `.venv` is stale in +the same way, and `uv sync` is what refreshes it. To get back to a clean environment: + +```bash +uv pip uninstall your-driver +uv sync --locked +``` + +## Related + +[`formats.md`](formats.md) is the same mechanism for exporters and the precedent this group +follows; [`providers.md`](providers.md) is the third entry-point group, for model drivers. +[`docs/content/preprocessing.md`](../../preprocessing.md) is the surface a user meets: what a +recipe is, how variants are seeded, and what each surface offers. diff --git a/docs/content/mcp-tools.md b/docs/content/mcp-tools.md index 15d78d1a..1fb01921 100644 --- a/docs/content/mcp-tools.md +++ b/docs/content/mcp-tools.md @@ -11,7 +11,7 @@ error envelope, and the three gate words. ## Always offered -56 tools, in the order an agent meets them: make a project, give it a schema, put images in it, work through them, promote, publish, export. +58 tools, in the order an agent meets them: make a project, give it a schema, put images in it, work through them, promote, publish, export. | Tool | Takes | What it does | | --- | --- | --- | @@ -63,7 +63,9 @@ error envelope, and the three gate words. | `check_export` | `project`, `tag`, `target`?, `format`?, `recipe`? | Say what a target or a format would drop from a release, without writing anything. | | `export_release` | `project`, `tag`, `dest`, `target`?, `format`?, `allow_lossy`?, `recipe`? | Write a release to a local directory, for a target or in one of the installed formats. | | `list_preprocessing_recipes` | `project` | List a project's pre-processing recipes, oldest first, each with its whole spec. | +| `get_preprocessing_recipe` | `project`, `name` | Read one pre-processing recipe by name, with its whole spec. | | `create_preprocessing_recipe` | `project`, `name`, `spec` | Store a named pre-processing recipe on a project, for `export_release` to apply. | +| `update_preprocessing_recipe` | `project`, `name`, `spec`, `new_name`? | Replace a pre-processing recipe's spec whole, and rename it when `new_name` is given. | | `list_inference_connections` | — | List this workspace's model connections, oldest first. | | `model_download_size` | `model_id`, `model_revision` | How big fetching that model's weights would be. Nothing is downloaded. | | `create_inference_connection` | `name`, `connection_type`, `model_id`, `model_revision`, `device`?, `precision`?, `endpoint_url`?, `provider_id`?, `credential_env`? | Configure a connection. Nothing is downloaded and nothing is contacted. | diff --git a/docs/content/mcp.md b/docs/content/mcp.md index 59ebf145..66671b5b 100644 --- a/docs/content/mcp.md +++ b/docs/content/mcp.md @@ -79,7 +79,7 @@ it, and what twelve real agent runs did with it - see ## The tools -Fifty-six tools are offered by default, in the order an agent meets them, plus the four +Fifty-eight tools are offered by default, in the order an agent meets them, plus the four below that are offered only on request — see [above](#destructive-tools-are-not-offered-unless-you-ask). [mcp-tools.md](mcp-tools.md) is the complete listing, generated from the server itself; this @@ -209,6 +209,8 @@ call until the end. #439 has since added a job gate, but it changes none of this | `export_release` | Write a release to a local directory, for a target or in a format. `allow_lossy` where needed; `recipe` applies a pre-processing recipe by name. | | `create_preprocessing_recipe` | Store a named [pre-processing recipe](preprocessing.md) on a project: a resize step, augmentation steps, and how many variants each train image gets. | | `list_preprocessing_recipes` | A project's recipes, each with its whole spec. `name` is what `export_release` takes as `recipe`. | +| `get_preprocessing_recipe` | One recipe by name, with its whole spec. | +| `update_preprocessing_recipe` | Replace a recipe's spec whole, and rename it with `new_name`. An export that already ran kept its own copy of the spec, so only the next export with that name is affected. | ### Inference connections @@ -363,7 +365,7 @@ The API's upload staging exists because HTTP has bytes where the kernel has path beside the workspace and has the filesystem. **One workspace per server.** No tool takes a workspace parameter — threading one through -fifty-six tools would put a path an agent has no way to know into every call. The workspace is +fifty-eight tools would put a path an agent has no way to know into every call. The workspace is opened and closed per tool call rather than held, so the file is never kept from `visionset server` or a second agent between calls. @@ -384,9 +386,9 @@ advertised only on request; the pre-labeling trio, `pre_label_job` beside the tw closing the last capability declared with no consumer; `check_export`, the plan-before-apply half of an export on the `preview_schema_change` precedent; `list_export_targets`, because `export_release` takes a target name and an agent has to be able to read the catalog it comes -from; and the three recipe tools, because `export_release` takes a recipe by name and an agent -has to be able to write one and read the names back. That is fifty-six offered by default and -sixty in all. The parity rule means +from; and the five recipe tools, because `export_release` takes a recipe by name and an agent +has to be able to write one, read it back and edit it on the same terms as REST and the CLI. That +is fifty-eight offered by default and sixty-two in all. The parity rule means *evaluated*, not *implemented* — tool-selection accuracy degrades with count, so a tool ships only when an agent has a reason to reach for it that no neighbour covers. diff --git a/docs/content/preprocessing.md b/docs/content/preprocessing.md index e19920c8..8d3ab206 100644 --- a/docs/content/preprocessing.md +++ b/docs/content/preprocessing.md @@ -98,8 +98,10 @@ other encoding a PNG. No metadata travels. ## Determinism, and its scope Variant `k` of an asset is seeded by `sha256(f"{recipe_hash}:{content_hash}:{k}")`, and every -draw - whether `hflip` mirrors, the brightness and contrast factors, how many quarter turns -`rot90` makes - reads a fixed position of that digest. The same recipe over the same bytes +draw - the brightness and contrast factors, how many quarter turns `rot90` makes - reads a fixed +position of that digest. `hflip` draws nothing and always mirrors, for the reason `rot90` never +draws zero turns: a variant that came out identical to its source would be the base image under a +variant's name. The same recipe over the same bytes therefore draws the same variant on any machine, and the geometry arithmetic is exact everywhere. @@ -172,8 +174,9 @@ scaled to match, and the response is never cached: the spec is the request's own | `PreprocessingRecipeNotFound` | The project has no recipe under that name. 404. | | `PreprocessingRecipeNameTaken` | Another recipe of the project carries that name, or a rename lands on one. Checked before writing and refused by a unique index, the `ReleaseTagTaken` shape. 409. | | `InvalidName` | The name is not a slug: lowercase letters, digits, dots, hyphens and underscores, starting with a letter or digit, at most 64 characters. 422. | +| `VALIDATION_ERROR` | The spec breaks the grammar: two resize steps, a resize after an augmentation, an augmentation repeated, augmentation steps with `variants_per_asset` 0, or variants with no augmentation step. The rule that was broken is the message, in the words above. Refused by the spec model itself, so every surface says the same thing. 422. | | `AugmentationRequiresSplit` | The recipe augments and the release was published without a split recipe. Raised at pre-flight and at export. 409. | -| `PreprocessingStepUnsupportedGeometry` | A step met a geometry it cannot transform - `rot90` over a polyline. Carries `step`, `geometry` and the first `asset_id`. 409. | +| `PreprocessingStepUnsupportedGeometry` | A step met a geometry outside the set it declares it can transform - every step declares `supported_geometries` the way an exporter does, and today the one exclusion is `rot90` over a polyline. Carries `step`, `geometry` and the first `asset_id`. 409. | | `ExportSourceUnreadable` | A step needs the source's pixel size and the manifest never recorded one, or the asset's bytes are gone. 409. | | `PreprocessingDriverNotFound` | No installed driver applies a step kind the recipe holds. A fact about the installation, not the request. 500. | @@ -184,6 +187,6 @@ scaled to match, and the response is never cached: the spec is the request's own | UI | The Dataset section's **Pre-processing** view: the project's recipes as a list, an editor of four steps (target model, resize, augmentation, preview), *Save recipe*, and a delete control on each row that asks first; the Export dialog's **Pre-processing recipe** control chooses one by name, `None` by default. See [ui.md](ui.md#the-dataset-its-releases-and-getting-the-data-out). | | REST | `POST`/`GET /projects/{id}/preprocessing-recipes`, `GET`/`PUT`/`DELETE /projects/{id}/preprocessing-recipes/{name}`, `POST /projects/{id}/preprocessing-preview`; `recipe=` on `POST /releases/{id}/export` and `GET /releases/{id}/export-compatibility`. The job carries the recipe as a snapshot. | | CLI | `visionset recipe create NAME -p P --spec FILE` or `--resize letterbox:640x640 --augment hflip,brightness_contrast --variants 2 --target yolo11`; `recipe list`, `show`, `update`, `delete`; `export --recipe NAME`. See [cli.md](cli.md#visionset-release-and-visionset-export). | -| MCP | `create_preprocessing_recipe`, `list_preprocessing_recipes`, `delete_preprocessing_recipe` (only with `--allow-destructive`); `recipe` on `export_release` and `check_export`. See [mcp.md](mcp.md#datasets-releases-and-export). | +| MCP | `create_preprocessing_recipe`, `list_preprocessing_recipes`, `get_preprocessing_recipe`, `update_preprocessing_recipe`, `delete_preprocessing_recipe` (only with `--allow-destructive`); `recipe` on `export_release` and `check_export`. See [mcp.md](mcp.md#datasets-releases-and-export). | The export half is described with the rest of exporting in [releases.md](releases.md#exporting). diff --git a/docs/content/tutorial.md b/docs/content/tutorial.md index c3305b1d..ad4d69e8 100644 --- a/docs/content/tutorial.md +++ b/docs/content/tutorial.md @@ -230,6 +230,10 @@ visionset export --project road-signs --release v1.0 \ --target yolo11 --out ./yolo --allow-lossy ``` +To resize every image, or write augmented variants of the train fold beside their sources, add +`--recipe NAME` naming a pre-processing recipe of the project - `--recipe letterbox-640` on the +command above; [preprocessing.md](preprocessing.md) is where a recipe is written. + `--allow-lossy` is required here and the refusal without it is not bureaucracy. A YOLO label row is a class index and coordinates, so attributes, confidence and provenance never survive. VisionSet works out exactly what that costs *before* writing anything, tells you by class with diff --git a/docs/src/sidebar.mjs b/docs/src/sidebar.mjs index f4382d04..4c54568f 100644 --- a/docs/src/sidebar.mjs +++ b/docs/src/sidebar.mjs @@ -49,6 +49,7 @@ export const sidebar = [ { slug: "architecture/backend/inference" }, { slug: "architecture/backend/providers" }, { slug: "architecture/backend/formats" }, + { slug: "architecture/backend/preprocessing-drivers" }, ], }, { diff --git a/src/visionset/kernel/domain/__init__.py b/src/visionset/kernel/domain/__init__.py index ae4bbd75..3624fa35 100644 --- a/src/visionset/kernel/domain/__init__.py +++ b/src/visionset/kernel/domain/__init__.py @@ -176,6 +176,8 @@ require_points_on_asset, ) from visionset.kernel.domain.preprocessing import ( + AUGMENT_GEOMETRIES, + EVERY_GEOMETRY, AugmentOp, AugmentStep, PreprocessingRecipe, @@ -183,7 +185,6 @@ ResizeStep, Step, brightness_contrast_factors, - hflip_applied, recipe_hash, rot90_quarter_turns, variant_seed, @@ -427,6 +428,8 @@ "TargetFamily", "Task", "TARGET_NAME_PATTERN", + "AUGMENT_GEOMETRIES", + "EVERY_GEOMETRY", "AugmentOp", "AugmentStep", "Fold", @@ -441,7 +444,6 @@ "TransformedFile", "TransformedView", "brightness_contrast_factors", - "hflip_applied", "letterbox_fit", "variant_content_hash", "source_of_content_hash", diff --git a/src/visionset/kernel/domain/preprocessing.py b/src/visionset/kernel/domain/preprocessing.py index 87c7ca5a..1d261f02 100644 --- a/src/visionset/kernel/domain/preprocessing.py +++ b/src/visionset/kernel/domain/preprocessing.py @@ -7,25 +7,38 @@ carry the same hash whatever order the fields were written in. Everything random about a variant is derived, never drawn: :func:`variant_seed` -turns ``(recipe, image, k)`` into a digest, and the three draw functions read -fixed positions of that digest. The geometry transform and the pixel driver -read the same positions, which is what keeps a variant's annotations on its -pixels. Byte stability is promised within one environment only; the geometry -arithmetic here is exact everywhere. +turns ``(recipe, image, k)`` into a digest, and the two draw functions read +fixed positions of that digest — ``hflip`` draws nothing, because a variant +that drew no mirror would be the base image under a variant's name. The +geometry transform and the pixel driver read the same positions, which is what +keeps a variant's annotations on its pixels. Byte stability is promised within +one environment only; the geometry arithmetic here is exact everywhere. + +Every step declares the geometries it can transform, the way an exporter +declares ``supported_geometries``: :data:`AUGMENT_GEOMETRIES` is the table per +augmentation, and each step reads it back as ``supported_geometries``. The +geometry transform refuses a manifest geometry outside that set, so a step's +reach is declared here once and never inferred from which branch of the +arithmetic happens to handle it. """ from __future__ import annotations import hashlib +from collections.abc import Mapping from datetime import datetime from enum import StrEnum -from typing import Annotated, Literal +from typing import Annotated, Final, Literal from uuid import UUID, uuid4 from pydantic import BaseModel, ConfigDict, Field, model_validator from visionset.kernel.domain.export_target import ResizeStrategy from visionset.kernel.domain.release import canonical_bytes, sha256_hex +from visionset.kernel.domain.schema import GeometryType + +EVERY_GEOMETRY: Final[frozenset[GeometryType]] = frozenset(GeometryType) +"""What a step that moves every coordinate the same way can transform.""" class ResizeStep(BaseModel): @@ -44,6 +57,16 @@ class ResizeStep(BaseModel): height: int = Field(ge=32, le=8192) pad_value: int = Field(default=114, ge=0, le=255) + @property + def name(self) -> str: + """What a refusal calls this step: its kind.""" + return self.kind + + @property + def supported_geometries(self) -> frozenset[GeometryType]: + """Every geometry: a resize scales and offsets each coordinate alike.""" + return EVERY_GEOMETRY + class AugmentOp(StrEnum): """An augmentation a recipe can apply when generating variants.""" @@ -53,12 +76,22 @@ class AugmentOp(StrEnum): ROT90 = "rot90" +AUGMENT_GEOMETRIES: Final[Mapping[AugmentOp, frozenset[GeometryType]]] = { + AugmentOp.HFLIP: EVERY_GEOMETRY, + AugmentOp.BRIGHTNESS_CONTRAST: EVERY_GEOMETRY, + # A polyline's point order carries meaning relative to the frame's axes, + # and a quarter turn re-axes the frame under it. + AugmentOp.ROT90: EVERY_GEOMETRY - {GeometryType.POLYLINE}, +} +"""Which geometries each augmentation can transform, per :class:`AugmentOp`.""" + + class AugmentStep(BaseModel): """One augmentation in a recipe. ``amount`` bounds the brightness and contrast factors — each is drawn uniformly from ``[1 - amount, 1 + amount]`` — and means nothing to - ``hflip`` or ``rot90``, whose draws have no magnitude. + ``hflip``, which always mirrors, or ``rot90``, whose draw has no magnitude. """ model_config = ConfigDict(frozen=True, extra="forbid") @@ -67,6 +100,16 @@ class AugmentStep(BaseModel): op: AugmentOp amount: float = Field(default=0.2, gt=0, le=0.5) + @property + def name(self) -> str: + """What a refusal calls this step: its augmentation op.""" + return self.op.value + + @property + def supported_geometries(self) -> frozenset[GeometryType]: + """What this augmentation can transform, read from :data:`AUGMENT_GEOMETRIES`.""" + return AUGMENT_GEOMETRIES[self.op] + Step = Annotated[ResizeStep | AugmentStep, Field(discriminator="kind")] """Every step a recipe can hold, discriminated the way ``Geometry`` is.""" @@ -151,17 +194,13 @@ def variant_seed(recipe_hash: str, content_hash: str, k: int) -> bytes: return hashlib.sha256(f"{recipe_hash}:{content_hash}:{k}".encode()).digest() -def hflip_applied(seed: bytes) -> bool: - """Whether this variant mirrors, read off bit 0 of the seed.""" - return bool(seed[0] & 1) - - def brightness_contrast_factors(seed: bytes, amount: float) -> tuple[float, float]: """This variant's brightness and contrast factors, in ``[1 - amount, 1 + amount]``. Brightness reads word 1 of the seed and contrast word 2 — fixed positions, whatever other steps the recipe holds, so adding a step never re-rolls the - others. + others. Word 0 is read by nothing; the positions here are load-bearing, + because moving one would re-roll every variant an export already wrote. """ return ( 1.0 - amount + 2.0 * amount * _fraction(seed, 1), diff --git a/src/visionset/kernel/domain/preprocessing_transform.py b/src/visionset/kernel/domain/preprocessing_transform.py index f134335d..8e2c0c4b 100644 --- a/src/visionset/kernel/domain/preprocessing_transform.py +++ b/src/visionset/kernel/domain/preprocessing_transform.py @@ -34,7 +34,6 @@ AugmentStep, RecipeSpec, ResizeStep, - hflip_applied, recipe_hash, rot90_quarter_turns, variant_seed, @@ -199,9 +198,9 @@ def transform_manifest( Raises: AugmentationRequiresSplit: the spec asks for variants and ``folds`` is ``None`` — the release was published without a split recipe. - PreprocessingStepUnsupportedGeometry: a step met a geometry it cannot - transform; today that is ``rot90`` over a polyline, whose point - order carries meaning relative to the frame's axes. + PreprocessingStepUnsupportedGeometry: a step met a geometry outside + its ``supported_geometries`` — ``rot90`` over a polyline, whose + point order carries meaning relative to the frame's axes. ExportSourceUnreadable: a step needs the source image's dimensions and the manifest never recorded them. """ @@ -275,15 +274,15 @@ def _variant_file( if resize is not None: geometries, width, height = _resized(asset, resize, geometries) for step in augments: + _refuse_unsupported(asset, step, geometries) if step.op is AugmentOp.HFLIP: - if hflip_applied(seed) and _any_coordinates(geometries): - mirror_width = float(_known_width(width, asset, step.op.value)) + if _any_coordinates(geometries): + mirror_width = float(_known_width(width, asset, step.name)) geometries = [_mirrored(geometry, mirror_width) for geometry in geometries] elif step.op is AugmentOp.ROT90: - _refuse_polylines(asset, geometries) for _ in range(rot90_quarter_turns(seed)): if _any_coordinates(geometries): - turn_width = float(_known_width(width, asset, step.op.value)) + turn_width = float(_known_width(width, asset, step.name)) geometries = [_rotated_once(geometry, turn_width) for geometry in geometries] width, height = height, width return TransformedFile( @@ -303,6 +302,7 @@ def _variant_file( def _resized( asset: ManifestAsset, step: ResizeStep, geometries: list[Geometry] ) -> tuple[list[Geometry], int, int]: + _refuse_unsupported(asset, step, geometries) if _any_coordinates(geometries): source_width, source_height = _dimensions(asset, step.kind) if step.strategy is ResizeStrategy.STRETCH: @@ -339,17 +339,19 @@ def _copied( ) -def _refuse_polylines(asset: ManifestAsset, geometries: Sequence[Geometry]) -> None: - if any(isinstance(geometry, PolylineGeometry) for geometry in geometries): - raise PreprocessingStepUnsupportedGeometry( - f"the 'rot90' step cannot transform a polyline (asset {asset.asset_id} carries " - "one): the path's point order carries meaning relative to the frame's axes, and " - "a quarter turn re-axes the frame under it. Remove the step, or export a release " - "without polylines", - step=AugmentOp.ROT90.value, - geometry="polyline", - asset_id=str(asset.asset_id), - ) +def _refuse_unsupported( + asset: ManifestAsset, step: ResizeStep | AugmentStep, geometries: Sequence[Geometry] +) -> None: + for geometry in geometries: + if geometry.type not in step.supported_geometries: + raise PreprocessingStepUnsupportedGeometry( + f"the {step.name!r} step cannot transform a {geometry.type.value} (asset " + f"{asset.asset_id} carries one). Remove the step, or export a release " + f"without {geometry.type.value} labels", + step=step.name, + geometry=geometry.type.value, + asset_id=str(asset.asset_id), + ) def _dimensions(asset: ManifestAsset, step: str) -> tuple[int, int]: diff --git a/src/visionset/kernel/ports/preprocessing.py b/src/visionset/kernel/ports/preprocessing.py index c83e0429..9b5df995 100644 --- a/src/visionset/kernel/ports/preprocessing.py +++ b/src/visionset/kernel/ports/preprocessing.py @@ -20,9 +20,9 @@ class PreprocessingDriver(Protocol): ``seed`` is the variant's digest from ``variant_seed`` and ``variant`` its index; a resize, or any step applied to variant 0, is deterministic and reads neither. Everything random must be derived from the seed through the - kernel's draw functions — ``hflip_applied``, - ``brightness_contrast_factors``, ``rot90_quarter_turns`` — so the pixels - land where the geometry transform already put the annotations. + kernel's draw functions — ``brightness_contrast_factors`` and + ``rot90_quarter_turns``; ``hflip`` draws nothing and always mirrors — so + the pixels land where the geometry transform already put the annotations. """ step_kinds: frozenset[str] diff --git a/src/visionset/mcp/main.py b/src/visionset/mcp/main.py index d2d2cda4..3c89b2f2 100644 --- a/src/visionset/mcp/main.py +++ b/src/visionset/mcp/main.py @@ -132,7 +132,9 @@ (releases.check_export, READS), (releases.export_release, WRITES), (preprocessing.list_preprocessing_recipes, READS), + (preprocessing.get_preprocessing_recipe, READS), (preprocessing.create_preprocessing_recipe, WRITES), + (preprocessing.update_preprocessing_recipe, WRITES), # After the cycle, not in it: connections are workspace configuration — # every project shares them — so they read as the appendix rather than as a # rung. Within the group, the order is the setup journey: see what is diff --git a/src/visionset/mcp/preprocessing.py b/src/visionset/mcp/preprocessing.py index 1bd5d04c..f193bca8 100644 --- a/src/visionset/mcp/preprocessing.py +++ b/src/visionset/mcp/preprocessing.py @@ -1,17 +1,16 @@ # usage: from visionset.mcp import preprocessing -"""Pre-processing recipe tools: name what an export does to its images, and list it. +"""Pre-processing recipe tools: create, list, read, update, delete. A recipe is a project resource an agent creates once and names on ``export_release`` and ``check_export``; the export keeps the spec by value, so nothing an agent does to a recipe afterwards changes an export that already ran. -``delete_preprocessing_recipe`` is offered only under ``--allow-destructive``, -with ``confirm``, on the terms every other delete follows — a recipe is small, -but it is shared, named work. - -There is no ``get`` and no ``update``: a project holds a handful of recipes and -the listing carries every field, and an agent that wants a different recipe -creates it under a new name rather than editing one somebody else may be -exporting with. +That is what makes ``update_preprocessing_recipe`` safe to offer: replacing a +spec, or renaming, touches the stored value and never a past export, and the +five tools are the same five operations REST and the CLI offer, so an agent +edits the recipe a person named rather than leaving a near-duplicate beside +it. ``delete_preprocessing_recipe`` is offered only under +``--allow-destructive``, with ``confirm``, on the terms every other delete +follows — a recipe is small, but it is shared, named work. """ from __future__ import annotations @@ -78,6 +77,49 @@ def create_preprocessing_recipe( return wire.preprocessing_recipe(created) +def get_preprocessing_recipe(project: ProjectRef, name: NameRef) -> dict[str, Any]: + """Read one pre-processing recipe by name, with its whole spec. + + The same row `list_preprocessing_recipes` shows, addressed by the name + `export_release` and `check_export` take as `recipe`. An unknown project or + recipe is reported as missing. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + found = PreprocessingRecipeService(workspace).get(resolved.id, name) + return wire.preprocessing_recipe(found) + + +def update_preprocessing_recipe( + project: ProjectRef, + name: NameRef, + spec: Annotated[ + RecipeSpec, + Field(description="The whole spec, replaced. Same shape as on create."), + ], + new_name: Annotated[ + str | None, + Field(description="Rename the recipe. Omit to keep its name."), + ] = None, +) -> dict[str, Any]: + """Replace a pre-processing recipe's spec whole, and rename it when `new_name` is given. + + Whole-value: the spec is one value with cross-field rules, so there is no + field-at-a-time edit; send every step. Nothing downstream moves — an export + that already ran kept its own copy of the spec, so editing changes only + what the next export with this name applies. Refuses an unknown project or + recipe, a `new_name` another recipe of the project already uses, a + `new_name` that is not a slug, and a spec that breaks the grammar + `create_preprocessing_recipe` describes. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + updated = PreprocessingRecipeService(workspace).update( + resolved.id, name, spec=spec, new_name=new_name + ) + return wire.preprocessing_recipe(updated) + + def list_preprocessing_recipes(project: ProjectRef) -> dict[str, Any]: """List a project's pre-processing recipes, oldest first, each with its whole spec. diff --git a/src/visionset/preprocessing/pillow/__init__.py b/src/visionset/preprocessing/pillow/__init__.py index ade30ed3..56244740 100644 --- a/src/visionset/preprocessing/pillow/__init__.py +++ b/src/visionset/preprocessing/pillow/__init__.py @@ -33,7 +33,6 @@ ResizeStrategy, Step, brightness_contrast_factors, - hflip_applied, letterbox_fit, rot90_quarter_turns, ) @@ -172,11 +171,12 @@ def _letterboxed(source: Image.Image, step: ResizeStep) -> Image.Image: class PillowAugmentDriver: """``augment`` steps: mirror, brightness then contrast, or a quarter turn. - Every draw comes from the kernel — ``hflip_applied``, - ``brightness_contrast_factors``, ``rot90_quarter_turns`` — over the seed - the caller passes, so the pixels land where the geometry transform put the - labels. Variant 0 is the base image and is returned untouched apart from - orientation and re-encoding, which is what a step applied to it means. + Every draw comes from the kernel — ``brightness_contrast_factors``, + ``rot90_quarter_turns``; a mirror is not drawn, every hflip variant + mirrors — over the seed the caller passes, so the pixels land where the + geometry transform put the labels. Variant 0 is the base image and is + returned untouched apart from orientation and re-encoding, which is what a + step applied to it means. """ step_kinds: frozenset[str] = frozenset({"augment"}) @@ -193,7 +193,7 @@ def apply(self, step: Step, image: bytes, *, seed: bytes, variant: int) -> bytes def _augmented(source: Image.Image, step: AugmentStep, seed: bytes) -> Image.Image: if step.op is AugmentOp.HFLIP: - return ImageOps.mirror(source) if hflip_applied(seed) else source + return ImageOps.mirror(source) if step.op is AugmentOp.BRIGHTNESS_CONTRAST: brightness, contrast = brightness_contrast_factors(seed, step.amount) brightened = ImageEnhance.Brightness(source).enhance(brightness) diff --git a/src/visionset/server/errors.py b/src/visionset/server/errors.py index d3e93c94..1cea4e5a 100644 --- a/src/visionset/server/errors.py +++ b/src/visionset/server/errors.py @@ -358,14 +358,12 @@ class ErrorRule: InferenceConnectionNotSetUp: ErrorRule(409, "INFERENCE_CONNECTION_NOT_SET_UP"), # An augmenting recipe against a release published without a split recipe. # Change-the-state-and-resubmit: publish a release with a split and the - # identical export succeeds. No route raises it yet — the recipe routes are - # not built — mapped for BATCH_IMMUTABLE's reason. + # identical export succeeds. AugmentationRequiresSplit: ErrorRule(409, "AUGMENTATION_REQUIRES_SPLIT"), # A recipe step meeting a geometry this release carries and the step cannot # move. LOSSY_EXPORT_NOT_CONSENTED's reading — a well-formed request refused # by the release's content — without the consent flag, because a label that - # cannot follow its image is never something to consent to. No route raises - # it yet; mapped for BATCH_IMMUTABLE's reason. + # cannot follow its image is never something to consent to. PreprocessingStepUnsupportedGeometry: ErrorRule(409, "PREPROCESSING_STEP_UNSUPPORTED_GEOMETRY"), # --- 422: the payload itself is wrong ---------------------------------- InvalidName: ErrorRule(422, "INVALID_NAME"), diff --git a/tests/kernel/test_preprocessing.py b/tests/kernel/test_preprocessing.py index dd086799..1ee1a521 100644 --- a/tests/kernel/test_preprocessing.py +++ b/tests/kernel/test_preprocessing.py @@ -9,14 +9,15 @@ from pydantic import ValidationError from visionset.kernel.domain import ( + AUGMENT_GEOMETRIES, AugmentOp, AugmentStep, + GeometryType, PreprocessingRecipe, RecipeSpec, ResizeStep, ResizeStrategy, brightness_contrast_factors, - hflip_applied, recipe_hash, rot90_quarter_turns, variant_seed, @@ -159,6 +160,20 @@ def test_the_hash_is_a_sha256_hex_digest_that_moves_with_the_content() -> None: # --- the draws -------------------------------------------------------------- +def test_every_step_declares_what_it_transforms_and_only_rot90_excludes_polylines() -> None: + assert RESIZE.supported_geometries == frozenset(GeometryType) + assert RESIZE.name == "resize" + for op in AugmentOp: + step = AugmentStep(op=op) + assert step.name == op.value + assert step.supported_geometries == AUGMENT_GEOMETRIES[op] + assert HFLIP.supported_geometries == frozenset(GeometryType) + assert AugmentStep(op=AugmentOp.BRIGHTNESS_CONTRAST).supported_geometries == frozenset( + GeometryType + ) + assert ROT90.supported_geometries == frozenset(GeometryType) - {GeometryType.POLYLINE} + + def test_a_variant_seed_is_deterministic_and_distinct_per_variant_and_image() -> None: seed = variant_seed("recipe", "image", 1) assert seed == variant_seed("recipe", "image", 1) @@ -168,11 +183,6 @@ def test_a_variant_seed_is_deterministic_and_distinct_per_variant_and_image() -> assert seed != variant_seed("other", "image", 1) -def test_hflip_reads_bit_zero_of_the_seed() -> None: - assert hflip_applied(bytes([0x01]) + bytes(31)) is True - assert hflip_applied(bytes([0xFE]) + bytes(31)) is False - - def test_brightness_and_contrast_read_words_one_and_two() -> None: lowest = bytes(4) + bytes(4) + bytes(4) + bytes(20) highest = bytes(4) + bytes([0xFF] * 4) + bytes([0xFF] * 4) + bytes(20) diff --git a/tests/kernel/test_preprocessing_transform.py b/tests/kernel/test_preprocessing_transform.py index 2c372973..39218dc7 100644 --- a/tests/kernel/test_preprocessing_transform.py +++ b/tests/kernel/test_preprocessing_transform.py @@ -12,11 +12,13 @@ import pytest from visionset.kernel.domain import ( + AUGMENT_GEOMETRIES, AugmentOp, AugmentStep, BboxGeometry, ClassificationGeometry, Geometry, + GeometryType, Manifest, ManifestAnnotation, ManifestAsset, @@ -27,7 +29,6 @@ ResizeStrategy, SplitAssignment, TransformedFile, - hflip_applied, letterbox_fit, recipe_hash, rot90_quarter_turns, @@ -93,16 +94,6 @@ def _geometry_of(file: TransformedFile, kind: type[Geometry]) -> Geometry: return match -def _content_hash_where_hflip(spec: RecipeSpec, *, applied: bool) -> str: - """A content hash whose variant-1 seed draws hflip the way the test wants.""" - spec_hash = recipe_hash(spec) - for candidate in range(1000): - content_hash = f"content-{candidate}" - if hflip_applied(variant_seed(spec_hash, content_hash, 1)) is applied: - return content_hash - raise AssertionError("a thousand seeds never drew the wanted bit") - - # --- letterbox arithmetic --------------------------------------------------- @@ -194,8 +185,8 @@ def test_without_a_resize_the_base_file_keeps_the_source_size() -> None: # --- augmentation × geometry ---------------------------------------------- -def _only_variant(spec: RecipeSpec, *geometries: Geometry, applied: bool = True) -> TransformedFile: - asset = _asset(*geometries, content_hash=_content_hash_where_hflip(spec, applied=applied)) +def _only_variant(spec: RecipeSpec, *geometries: Geometry) -> TransformedFile: + asset = _asset(*geometries) view = transform_manifest(_manifest(asset), spec, _train(asset)) (variant,) = [file for file in view.files if file.variant == 1] return variant @@ -203,7 +194,7 @@ def _only_variant(spec: RecipeSpec, *geometries: Geometry, applied: bool = True) def test_hflip_mirrors_in_the_frame_width_and_keeps_polyline_order() -> None: spec = _spec(AugmentStep(op=AugmentOp.HFLIP), variants=1) - variant = _only_variant(spec, *EVERY_GEOMETRY, applied=True) + variant = _only_variant(spec, *EVERY_GEOMETRY) assert (variant.width, variant.height) == (100, 200) assert _geometry_of(variant, BboxGeometry) == BboxGeometry( @@ -218,11 +209,15 @@ def test_hflip_mirrors_in_the_frame_width_and_keeps_polyline_order() -> None: assert _geometry_of(variant, ClassificationGeometry) == TAG -def test_hflip_not_drawn_leaves_the_variant_unmirrored() -> None: +@pytest.mark.parametrize("content_hash", [f"content-{candidate}" for candidate in range(8)]) +def test_an_hflip_variant_is_never_its_source(content_hash: str) -> None: + """Whatever the seed draws, a variant mirrors: an unmirrored one would copy the base image.""" spec = _spec(AugmentStep(op=AugmentOp.HFLIP), variants=1) - variant = _only_variant(spec, BBOX, POLYLINE, applied=False) - assert _geometry_of(variant, BboxGeometry) == BBOX - assert _geometry_of(variant, PolylineGeometry) == POLYLINE + asset = _asset(BBOX, POLYLINE, content_hash=content_hash) + view = transform_manifest(_manifest(asset), spec, _train(asset)) + (variant,) = [file for file in view.files if file.variant == 1] + assert _geometry_of(variant, BboxGeometry) != BBOX + assert _geometry_of(variant, PolylineGeometry) != POLYLINE def test_brightness_contrast_changes_no_geometry_and_no_size() -> None: @@ -271,6 +266,53 @@ def test_rot90_refuses_a_polyline_and_names_the_step_geometry_and_asset() -> Non assert caught.value.asset_id == str(asset.asset_id) +@pytest.mark.parametrize("op", list(AugmentOp)) +@pytest.mark.parametrize("geometry", EVERY_GEOMETRY, ids=lambda geometry: geometry.type.value) +def test_a_step_transforms_exactly_the_geometries_it_declares( + op: AugmentOp, geometry: Geometry +) -> None: + step = AugmentStep(op=op) + spec = _spec(step, variants=1) + asset = _asset(geometry) + + if geometry.type in step.supported_geometries: + view = transform_manifest(_manifest(asset), spec, _train(asset)) + (variant,) = [file for file in view.files if file.variant == 1] + assert variant.annotations[0].geometry.type is geometry.type + return + with pytest.raises(PreprocessingStepUnsupportedGeometry) as caught: + transform_manifest(_manifest(asset), spec, _train(asset)) + assert (caught.value.step, caught.value.geometry) == (op.value, geometry.type.value) + assert caught.value.asset_id == str(asset.asset_id) + + +def test_the_refusal_reads_the_declaration_not_the_arithmetic( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Narrowing hflip's declared geometries makes it refuse a box it can mirror.""" + narrowed = AUGMENT_GEOMETRIES[AugmentOp.HFLIP] - {GeometryType.BBOX} + monkeypatch.setitem(AUGMENT_GEOMETRIES, AugmentOp.HFLIP, narrowed) + spec = _spec(AugmentStep(op=AugmentOp.HFLIP), variants=1) + asset = _asset(BBOX) + + with pytest.raises(PreprocessingStepUnsupportedGeometry) as caught: + transform_manifest(_manifest(asset), spec, _train(asset)) + assert (caught.value.step, caught.value.geometry) == ("hflip", "bbox") + + +def test_a_resize_consults_its_declaration_on_the_base_file( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + ResizeStep, "supported_geometries", property(lambda step: frozenset({GeometryType.BBOX})) + ) + step = ResizeStep(strategy=ResizeStrategy.STRETCH, width=64, height=64) + + with pytest.raises(PreprocessingStepUnsupportedGeometry) as caught: + transform_manifest(_manifest(_asset(POLYGON)), _spec(step), None) + assert (caught.value.step, caught.value.geometry) == ("resize", "polygon") + + def test_rot90_leaves_a_polyline_outside_the_train_fold_alone() -> None: spec = _spec(AugmentStep(op=AugmentOp.ROT90), variants=1) asset = _asset(POLYLINE) @@ -288,7 +330,7 @@ def test_rot90_swaps_the_size_of_a_tag_only_asset_without_needing_coordinates() def test_steps_compose_in_recipe_order_resize_first() -> None: resize = ResizeStep(strategy=ResizeStrategy.STRETCH, width=200, height=100) spec = _spec(resize, AugmentStep(op=AugmentOp.HFLIP), variants=1) - variant = _only_variant(spec, BBOX, applied=True) + variant = _only_variant(spec, BBOX) # Stretched to (20, 10, 60, 20) in a 200-wide frame, then mirrored: x = 200 − 20 − 60. assert (variant.width, variant.height) == (200, 100) assert _geometry_of(variant, BboxGeometry) == BboxGeometry( diff --git a/tests/mcp/test_preprocessing_tools.py b/tests/mcp/test_preprocessing_tools.py index 1dfc5a06..c2830394 100644 --- a/tests/mcp/test_preprocessing_tools.py +++ b/tests/mcp/test_preprocessing_tools.py @@ -49,6 +49,64 @@ def test_a_taken_name_and_a_broken_spec_are_refused(named: str) -> None: assert broken.is_error +def test_a_recipe_is_read_back_by_name_and_a_missing_one_is_refused(named: str) -> None: + created = payload(call("create_preprocessing_recipe", project=named, name="lb", spec=LETTERBOX)) + + assert payload(call("get_preprocessing_recipe", project=named, name="lb")) == created + missing = error(call("get_preprocessing_recipe", project=named, name="nope")) + assert "no pre-processing recipe named 'nope'" in missing["message"] + + +def test_update_replaces_the_spec_whole_and_renames_on_request(named: str) -> None: + created = payload(call("create_preprocessing_recipe", project=named, name="lb", spec=LETTERBOX)) + + replaced = payload(call("update_preprocessing_recipe", project=named, name="lb", spec=FLIPS)) + assert replaced["id"] == created["id"] + assert replaced["name"] == "lb" + assert replaced["spec"]["steps"] == [{"kind": "augment", "op": "hflip", "amount": 0.2}] + assert replaced["spec"]["variants_per_asset"] == 1 + + renamed = payload( + call("update_preprocessing_recipe", project=named, name="lb", spec=FLIPS, new_name="flips") + ) + assert renamed["name"] == "flips" + assert [ + one["name"] for one in payload(call("list_preprocessing_recipes", project=named))["items"] + ] == ["flips"] + assert payload(call("get_preprocessing_recipe", project=named, name="flips")) == renamed + + +def test_update_refuses_a_missing_recipe_a_taken_name_and_a_broken_spec(named: str) -> None: + payload(call("create_preprocessing_recipe", project=named, name="lb", spec=LETTERBOX)) + payload(call("create_preprocessing_recipe", project=named, name="flips", spec=FLIPS)) + + missing = error(call("update_preprocessing_recipe", project=named, name="nope", spec=FLIPS)) + assert "no pre-processing recipe named 'nope'" in missing["message"] + taken = error( + call( + "update_preprocessing_recipe", + project=named, + name="lb", + spec=LETTERBOX, + new_name="flips", + ) + ) + assert "already has a pre-processing recipe named 'flips'" in taken["message"] + broken = call( + "update_preprocessing_recipe", + project=named, + name="lb", + spec={**LETTERBOX, "variants_per_asset": 2}, + ) + assert broken.is_error + unchanged = payload(call("get_preprocessing_recipe", project=named, name="lb")) + assert ( + unchanged["spec"] + == payload(call("list_preprocessing_recipes", project=named))["items"][0]["spec"] + ) + assert unchanged["spec"]["steps"][0]["kind"] == "resize" + + def test_delete_is_offered_only_on_request_and_takes_confirm(named: str) -> None: payload(call("create_preprocessing_recipe", project=named, name="lb", spec=LETTERBOX)) assert "delete_preprocessing_recipe" not in tool_names() diff --git a/tests/mcp/test_registration.py b/tests/mcp/test_registration.py index 92c24b81..7379409b 100644 --- a/tests/mcp/test_registration.py +++ b/tests/mcp/test_registration.py @@ -68,7 +68,9 @@ "export_release", "list_export_targets", "list_preprocessing_recipes", + "get_preprocessing_recipe", "create_preprocessing_recipe", + "update_preprocessing_recipe", "list_formats", "list_inference_connections", "model_download_size", diff --git a/tests/preprocessing/test_pillow_drivers.py b/tests/preprocessing/test_pillow_drivers.py index 3e8ed5f5..a94ec079 100644 --- a/tests/preprocessing/test_pillow_drivers.py +++ b/tests/preprocessing/test_pillow_drivers.py @@ -17,7 +17,6 @@ ResizeStep, ResizeStrategy, brightness_contrast_factors, - hflip_applied, letterbox_fit, rot90_quarter_turns, variant_seed, @@ -235,16 +234,17 @@ def test_variant_zero_is_the_base_image_re_encoded() -> None: assert _white_pixels(out) == {(3, 7)} -def test_hflip_mirrors_exactly_when_the_kernel_says_so() -> None: +@pytest.mark.parametrize("k", [1, 2, 3]) +def test_hflip_mirrors_every_variant_whatever_the_seed(k: int) -> None: step = AugmentStep(op=AugmentOp.HFLIP) - flipping = _seed_where(hflip_applied) - keeping = _seed_where(lambda seed: not hflip_applied(seed)) + source = _marked((40, 30), (3, 7)) + + seed = variant_seed("recipe", "content", k) - flipped = _open(AUGMENT.apply(step, _marked((40, 30), (3, 7)), seed=flipping, variant=1)) - kept = _open(AUGMENT.apply(step, _marked((40, 30), (3, 7)), seed=keeping, variant=1)) + data = AUGMENT.apply(step, source, seed=seed, variant=k) - assert _white_pixels(flipped) == {(40 - 1 - 3, 7)} - assert _white_pixels(kept) == {(3, 7)} + assert data != source + assert _white_pixels(_open(data)) == {(40 - 1 - 3, 7)} @pytest.mark.parametrize("turns", [1, 2, 3])