From 7fcbe6442553970a2fcdc10de340008cb6bd6bf2 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Tue, 28 Jul 2026 21:04:55 -0700 Subject: [PATCH 1/3] refactor(wire): promote the JSON projections out of the CLI `cli/_json.py` becomes `visionset/wire/`, a surface-agnostic package the CLI and (next) MCP both import. A second hand-written spelling of the same twenty shapes is what "promoted, not copied" exists to prevent. The direction stays one-way and machine-enforced: `visionset.wire` joins the kernel-purity contract's forbidden list beside the three delivery packages. The server keeps its pydantic models, because `openapi.json` is generated from them. Six projections the CLI never needed and MCP does: `schema_change`, `schema_diff`, `batch_asset`, `geometry`, `annotation`, `class_count` and `dataset_stats`. Eleven new pairs in the parity gate, plus `dataset` and `asset_progress`, which had projections but no row. `schema_diff` has no route behind it, so it joins the two encoding-only shapes at the bottom. No behaviour change: 1698 tests, up from 1670 by exactly the new parity cases. --- docs/cli.md | 14 +- docs/schemas.md | 6 +- pyproject.toml | 11 ++ src/visionset/cli/_output.py | 4 +- src/visionset/cli/batches.py | 12 +- src/visionset/cli/export.py | 4 +- src/visionset/cli/formats.py | 4 +- src/visionset/cli/ingest.py | 8 +- src/visionset/cli/jobs.py | 14 +- src/visionset/cli/projects.py | 6 +- src/visionset/cli/releases.py | 8 +- src/visionset/cli/schemas.py | 6 +- .../{cli/_json.py => wire/__init__.py} | 175 ++++++++++++++++-- tests/cli/test_json_contract.py | 100 ++++++---- tests/fixtures/samples.py | 63 ++++++- uv.lock | 2 + 16 files changed, 341 insertions(+), 96 deletions(-) rename src/visionset/{cli/_json.py => wire/__init__.py} (61%) diff --git a/docs/cli.md b/docs/cli.md index 5a4c5b34..2586b5dc 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -364,15 +364,15 @@ its target by import string or subprocess for the same reason `ui` does — impo ## For contributors -Five private modules carry everything a command needs: +Four private modules and one shared package carry everything a command needs: | | | | --- | --- | | `cli/_errors.py` | the exit codes and `domain_errors()` | | `cli/_workspace.py` | `WorkspaceOption` and `opened_workspace()` | | `cli/_output.py` | `JsonOption`, the column formatter, `document()`, `note()` | -| `cli/_json.py` | one hand-written projection per resource | | `cli/_resolve.py` | `ProjectOption`, and turning a name or a tag into the thing it names | +| `visionset/wire/` | one hand-written projection per resource — **shared with the MCP surface**, which publishes the same shapes (see `docs/mcp.md`) | A new command is a module beside them and one registration line in `cli/main.py` — groups by `add_typer`, bare commands by `app.command("name")(fn)`, which is where they are registered rather @@ -384,10 +384,12 @@ close and the refusal, and it closes in a `finally` so no `visionset.db-wal` is **A command maps to exactly one service call, and says so in its docstring when it does not.** `ingest` is the only one that does not, and its module explains why. -**Never `model_dump()` a domain model into `--json`.** Write the projection in `_json.py` and add -the pair to `tests/cli/test_json_contract.py`, which asserts key-for-key parity with the REST wire -model. That test may import both `visionset.cli` and `visionset.server` because `tests/` is outside -the package the independence contract governs — the packages themselves must not. +**Never `model_dump()` a domain model into `--json`.** Write the projection in `visionset/wire/` +and add the pair to `tests/cli/test_json_contract.py`, which asserts key-for-key parity with the +REST wire model. That test may import both `visionset.wire` and `visionset.server` because `tests/` +is outside the package the independence contract governs — the packages themselves must not. A +projection added there is published by the CLI **and** by MCP, which is why it is a package of its +own rather than a private module under `cli/`. **A bound the domain enforces with a pydantic `Field` has to be mirrored in the Typer option**, or the refusal arrives as a traceback: a pydantic `ValidationError` and a bare `ValueError` are not diff --git a/docs/schemas.md b/docs/schemas.md index 6ef11a8f..c50a8b9c 100644 --- a/docs/schemas.md +++ b/docs/schemas.md @@ -226,9 +226,9 @@ The file is **JSON**, and it is byte-for-byte the same document } ``` -That is a tested claim rather than a promise: `tests/cli/test_json_contract.py` asserts the CLI's -`label_class` and `attribute` projections have exactly `LabelClassBody`'s and `AttributeBody`'s -fields, and `tests/cli/test_schemas.py` validates the example document as a request body. +That is a tested claim rather than a promise: `tests/cli/test_json_contract.py` asserts that +`visionset.wire`'s `label_class` and `attribute` projections have exactly `LabelClassBody`'s and +`AttributeBody`'s fields, and `tests/cli/test_schemas.py` validates the example document as a request body. **JSON and not YAML.** A second format means a runtime dependency in every wheel, a second parser to keep honest, and two shapes that can disagree — while the surface a schema file has to diff --git a/pyproject.toml b/pyproject.toml index 8b6e7740..8c878a90 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,6 +54,12 @@ dummy = "visionset.formats._dummy:DummyExporter" dev = [ "pytest>=8.2", "httpx>=0.27", + # The MCP tests drive the real protocol through an in-memory client session, + # which is async; `anyio.run` is the bridge that keeps every test a plain + # sync function and the suite free of a pytest asyncio plugin. Installed + # already as a transitive dependency of `mcp`, `httpx` and `starlette` — + # which is exactly why it is declared, the `python-multipart` reason. + "anyio>=4.5", "ruff>=0.5", "mypy>=1.10", "import-linter>=2.0", @@ -123,6 +129,11 @@ forbidden_modules = [ "visionset.cli", "visionset.mcp", "visionset.formats", + # The JSON shapes the surfaces publish. Not a delivery mechanism, but the same + # direction: it is written in terms of domain models, so the kernel importing + # it would close a loop and make a publication decision reachable from the + # place that decides what exists. + "visionset.wire", "fastapi", "typer", "mcp", diff --git a/src/visionset/cli/_output.py b/src/visionset/cli/_output.py index f024e64f..03f80646 100644 --- a/src/visionset/cli/_output.py +++ b/src/visionset/cli/_output.py @@ -47,7 +47,7 @@ TIMESTAMP_FORMAT: Final = "%Y-%m-%dT%H:%M:%SZ" """Seconds, UTC, no offset. A listing is read by a person; microseconds are not. -Deliberately **not** the format ``_json.py`` uses. That one has to agree with +Deliberately **not** the format ``visionset.wire`` uses. That one has to agree with pydantic's, because the JSON shapes are gated against the server's wire models; this one has to be readable in a column. Sharing them would break the parity gate in a way key-set comparison cannot see. @@ -93,7 +93,7 @@ def document(payload: Mapping[str, Any]) -> None: ``| jq '.items[]'`` works and a partial read is never mistaken for a whole one. ``json.dumps`` is called with **no** ``default=``, on purpose. A ``UUID``, a - ``datetime`` or a ``Path`` reaching this function is a projection in ``_json`` + ``datetime`` or a ``Path`` reaching this function is a projection in ``visionset.wire`` that forgot to encode a leaf, and that must be a ``TypeError`` a test catches rather than a silent ``str()`` nobody re-reads. """ diff --git a/src/visionset/cli/batches.py b/src/visionset/cli/batches.py index 9e8d9d9a..26ddfb01 100644 --- a/src/visionset/cli/batches.py +++ b/src/visionset/cli/batches.py @@ -30,7 +30,7 @@ import typer -from visionset.cli import _json +from visionset import wire from visionset.cli._output import JsonOption, document, note, table from visionset.cli._resolve import ProjectOption, resolve_project from visionset.cli._workspace import WorkspaceOption, opened_workspace @@ -79,7 +79,7 @@ def batch_list( # do not say whether anybody has started on it. counts = [jobs.batch_progress(batch.id) for batch in batches] if json_out: - document(_json.page([_json.batch(b, c) for b, c in zip(batches, counts, strict=True)])) + document(wire.page([wire.batch(b, c) for b, c in zip(batches, counts, strict=True)])) return table( _COLUMNS, @@ -130,7 +130,7 @@ def batch_approve( counts = JobService(service).batch_progress(approved.id) job_count = len(batches.jobs(approved.id)) if json_out: - document(_json.batch(approved, counts)) + document(wire.batch(approved, counts)) return note( f"Approved batch {approved.name!r} against schema version " @@ -149,7 +149,7 @@ def batch_start( with opened_workspace(workspace) as service: started = BatchService(service).start(batch) counts = JobService(service).batch_progress(started.id) - _echo(started.id, started.state.value, json_out, _json.batch(started, counts)) + _echo(started.id, started.state.value, json_out, wire.batch(started, counts)) @batch_app.command("complete") @@ -166,7 +166,7 @@ def batch_complete( with opened_workspace(workspace) as service: completed = BatchService(service).complete(batch) counts = JobService(service).batch_progress(completed.id) - _echo(completed.id, completed.state.value, json_out, _json.batch(completed, counts)) + _echo(completed.id, completed.state.value, json_out, wire.batch(completed, counts)) @batch_app.command("promote") @@ -184,7 +184,7 @@ def batch_promote( with opened_workspace(workspace) as service: promoted = DatasetService(service).promote(batch, actor=_ACTOR) if json_out: - document(_json.page([_json.asset(a) for a in promoted])) + document(wire.page([wire.asset(a) for a in promoted])) return note(f"Promoted {len(promoted)} asset(s) into the dataset.") for asset in promoted: diff --git a/src/visionset/cli/export.py b/src/visionset/cli/export.py index 4ad5e027..70d40aed 100644 --- a/src/visionset/cli/export.py +++ b/src/visionset/cli/export.py @@ -28,7 +28,7 @@ import typer -from visionset.cli import _json +from visionset import wire from visionset.cli._output import JsonOption, document, note from visionset.cli._resolve import ProjectOption, resolve_release from visionset.cli._workspace import WorkspaceOption, opened_workspace @@ -77,7 +77,7 @@ def export( found = resolve_release(service, project, release) result = ReleaseService(service).export(found.id, plugin, out, allow_lossy=allow_lossy) if json_out: - document(_json.export_result(result)) + document(wire.export_result(result)) return note( f"Exported {found.tag!r} as {result.format_name}: " diff --git a/src/visionset/cli/formats.py b/src/visionset/cli/formats.py index bb0fa932..5b973377 100644 --- a/src/visionset/cli/formats.py +++ b/src/visionset/cli/formats.py @@ -22,7 +22,7 @@ import typer -from visionset.cli import _json +from visionset import wire from visionset.cli._output import JsonOption, document, note, table from visionset.formats.registry import exporters @@ -37,7 +37,7 @@ def format_list(json_out: JsonOption = False) -> None: found = exporters() installed = [found[name] for name in sorted(found)] if json_out: - document(_json.page([_json.export_format(p) for p in installed])) + document(wire.page([wire.export_format(p) for p in installed])) return table(_COLUMNS, [(p.format_name, "yes" if p.lossy else "no") for p in installed]) if not installed: diff --git a/src/visionset/cli/ingest.py b/src/visionset/cli/ingest.py index 492dcf4f..6200c083 100644 --- a/src/visionset/cli/ingest.py +++ b/src/visionset/cli/ingest.py @@ -40,7 +40,7 @@ import typer -from visionset.cli import _json +from visionset import wire from visionset.cli._output import JsonOption, document, note, table from visionset.cli._resolve import ProjectOption, resolve_project from visionset.cli._workspace import WorkspaceOption, opened_workspace @@ -131,13 +131,13 @@ def ingest( if json_out: document( { - "source": _json.source(registered), + "source": wire.source(registered), "job_id": str(result.job_id), "batch_id": str(result.batch_id), "created": result.created, "deduplicated": result.deduplicated, "failed": result.failed, - "failures": [_json.ingest_failure(f) for f in result.failures], + "failures": [wire.ingest_failure(f) for f in result.failures], } ) return @@ -165,7 +165,7 @@ def backfill_thumbnails( resolved = resolve_project(service, project) report = IngestService(service).backfill_thumbnails(resolved.id) if json_out: - document(_json.thumbnail_backfill(report)) + document(wire.thumbnail_backfill(report)) return note( f"Examined {report.examined} asset(s) without a preview in {resolved.name!r}: " diff --git a/src/visionset/cli/jobs.py b/src/visionset/cli/jobs.py index c2afdf05..1a484c43 100644 --- a/src/visionset/cli/jobs.py +++ b/src/visionset/cli/jobs.py @@ -29,7 +29,7 @@ import typer -from visionset.cli import _json +from visionset import wire from visionset.cli._output import JsonOption, document, note, table from visionset.cli._workspace import WorkspaceOption, opened_workspace from visionset.kernel.domain import AssetProgress @@ -61,7 +61,7 @@ def job_list( with opened_workspace(workspace) as service: jobs = BatchService(service).jobs(batch) if json_out: - document(_json.page([_json.job(j, batch_id=batch) for j in jobs])) + document(wire.page([wire.job(j, batch_id=batch) for j in jobs])) return table(_COLUMNS, [(str(j.id), j.state.value, str(len(j.progress))) for j in jobs]) if not jobs: @@ -88,7 +88,7 @@ def job_next( with opened_workspace(workspace) as service: assets = JobService(service).next_pending(job, count) if json_out: - document(_json.page([_json.asset(a) for a in assets])) + document(wire.page([wire.asset(a) for a in assets])) return table( _ASSET_COLUMNS, @@ -116,7 +116,7 @@ def job_progress( with opened_workspace(workspace) as service: counts = JobService(service).job_progress(job) if json_out: - document(_json.progress_counts(counts)) + document(wire.progress_counts(counts)) return table( _PROGRESS_COLUMNS, @@ -136,7 +136,7 @@ def job_start( started = service_jobs.start(job) batch = service_jobs.batch(started.id) if json_out: - document(_json.job(started, batch_id=batch.id)) + document(wire.job(started, batch_id=batch.id)) return note(f"Job {started.id} is now {started.state.value}.") typer.echo(str(started.id)) @@ -162,7 +162,7 @@ def job_mark( with opened_workspace(workspace) as service: JobService(service).mark(job, asset, progress) if json_out: - document(_json.asset_progress(asset, progress)) + document(wire.asset_progress(asset, progress)) return note(f"Asset {asset} is now {progress.value}.") @@ -184,7 +184,7 @@ def job_complete( completed = service_jobs.complete(job) batch = service_jobs.batch(completed.id) if json_out: - document(_json.job(completed, batch_id=batch.id)) + document(wire.job(completed, batch_id=batch.id)) return note(f"Job {completed.id} is now {completed.state.value}.") typer.echo(str(completed.id)) diff --git a/src/visionset/cli/projects.py b/src/visionset/cli/projects.py index 1d8ca0ab..962c5ed0 100644 --- a/src/visionset/cli/projects.py +++ b/src/visionset/cli/projects.py @@ -23,7 +23,7 @@ import typer -from visionset.cli import _json +from visionset import wire from visionset.cli._output import JsonOption, document, note, table from visionset.cli._workspace import WorkspaceOption, opened_workspace from visionset.kernel.services import ProjectService @@ -49,7 +49,7 @@ def project_create( with opened_workspace(workspace) as service: created = ProjectService(service).create(name, description) if json_out: - document(_json.project(created)) + document(wire.project(created)) return note(f"Created project {created.name!r}.") typer.echo(str(created.id)) @@ -65,7 +65,7 @@ def project_list( projects = ProjectService(service).list() root = service.root if json_out: - document(_json.page([_json.project(p) for p in projects])) + document(wire.page([wire.project(p) for p in projects])) return table(_COLUMNS, [(str(p.id), p.name, p.description or _NONE) for p in projects]) if not projects: diff --git a/src/visionset/cli/releases.py b/src/visionset/cli/releases.py index 9e0984f4..901851b1 100644 --- a/src/visionset/cli/releases.py +++ b/src/visionset/cli/releases.py @@ -27,7 +27,7 @@ import typer -from visionset.cli import _json +from visionset import wire from visionset.cli._errors import EXIT_ANSWER_IS_NO from visionset.cli._output import JsonOption, document, moment, note, table from visionset.cli._resolve import ProjectOption, resolve_project, resolve_release @@ -96,7 +96,7 @@ def release_publish( dataset = ProjectService(service).get_dataset(resolved.id) published = ReleaseService(service).publish(dataset.id, tag, split=recipe) if json_out: - document(_json.release(published)) + document(wire.release(published)) return note( f"Published {published.tag!r}: {published.asset_count} asset(s), " @@ -118,7 +118,7 @@ def release_list( dataset = ProjectService(service).get_dataset(resolved.id) releases = ReleaseService(service).list(dataset.id) if json_out: - document(_json.page([_json.release(r) for r in releases])) + document(wire.page([wire.release(r) for r in releases])) return table( _COLUMNS, @@ -158,7 +158,7 @@ def release_verify( release = resolve_release(service, project, tag) report = ReleaseService(service).verify(release.id) if json_out: - document(_json.release_verification(report)) + document(wire.release_verification(report)) elif report.ok: note(f"Release {release.tag!r} verifies: {report.checked} blob(s) intact.") else: diff --git a/src/visionset/cli/schemas.py b/src/visionset/cli/schemas.py index d2912cc7..29a79da8 100644 --- a/src/visionset/cli/schemas.py +++ b/src/visionset/cli/schemas.py @@ -39,7 +39,7 @@ import typer from pydantic import TypeAdapter, ValidationError -from visionset.cli import _json +from visionset import wire from visionset.cli._output import JsonOption, document, note, table from visionset.cli._resolve import ProjectOption, resolve_project from visionset.cli._workspace import WorkspaceOption, opened_workspace @@ -111,7 +111,7 @@ def schema_apply( resolved.id, classes, allow_destructive=allow_destructive ) if json_out: - document(_json.schema_version(version)) + document(wire.schema_version(version)) return note(f"Applied schema version {version.version} to {resolved.name!r}.") typer.echo(str(version.version)) @@ -128,7 +128,7 @@ def schema_list( resolved = resolve_project(service, project) versions = SchemaService(service).list_versions(resolved.id) if json_out: - document(_json.page([_json.schema_version(v) for v in versions])) + document(wire.page([wire.schema_version(v) for v in versions])) return table( _COLUMNS, diff --git a/src/visionset/cli/_json.py b/src/visionset/wire/__init__.py similarity index 61% rename from src/visionset/cli/_json.py rename to src/visionset/wire/__init__.py index 226ea904..f89d8794 100644 --- a/src/visionset/cli/_json.py +++ b/src/visionset/wire/__init__.py @@ -1,31 +1,41 @@ -# usage: from visionset.cli import _json -"""What ``--json`` publishes: one hand-written projection per resource. +# usage: from visionset import wire +"""What a surface publishes as JSON: one hand-written projection per resource. -**A field reaches a script because somebody wrote it here.** That is +**A field reaches a caller because somebody wrote it here.** That is ``tokens.py``'s rule — its listing names three columns one at a time rather than dumping the model — promoted to the shape a program parses. The alternative, ``model_dump()`` on a domain model, would publish whatever the domain happens to hold today and silently republish whatever it holds tomorrow. Three fields make the point, and each is already absent from the wire model the server publishes: -- ``Asset.uri`` and ``Source.path`` are absolute paths on this machine. A script +- ``Asset.uri`` and ``Source.path`` are absolute paths on this machine. A caller reading them learns the layout of somebody's disk and nothing it can use. - ``Batch.asset_ids`` is a batch's whole roll call, which for fifty thousand frames must not travel on every read of its name. +**This module is its own package rather than ``cli/_json.py`` because it has two +callers.** It arrived with the CLI (#34) and #35 gave MCP the same need; a second +hand-written spelling of the same twenty shapes is exactly what "promoted, not +copied" exists to prevent, and the two would have been free to drift with only +prose holding them together. The import direction is one-way and machine-enforced: +the surfaces import ``visionset.wire``, and the kernel-purity contract forbids +``visionset.kernel`` importing it, alongside the three delivery packages. The +server keeps its own pydantic models because ``openapi.json`` is generated from +them, which a dict cannot do. + **These shapes deliberately agree, key for key, with the REST API's wire models.** -Not by importing them — import-linter forbids ``visionset.cli`` importing -``visionset.server``, and rightly: the surfaces are siblings — but by -``tests/cli/test_json_contract.py``, which imports both and asserts each pair has -the same keys *and* that the projection round-trips through the wire model. A -test may do what neither package may. What that buys is one shape for one -concept, so a script moves between ``curl | jq`` and ``visionset --json | jq`` -without relearning the field names. - -Two things here have no wire partner and are the CLI's own: an export report -(the API returns the archive itself, not a description of it) and a thumbnail -backfill (no route reaches it). Both are named in the docs as the CLI defining -the shape first, for #35 to follow. +Not by importing them — import-linter's independence contract keeps the surfaces +siblings — but by ``tests/cli/test_json_contract.py``, which imports both and +asserts each pair has the same keys *and* that the projection round-trips through +the wire model. A test may do what neither package may. What that buys is one +shape for one concept, so a caller moves between ``curl | jq``, +``visionset --json | jq`` and an MCP tool result without relearning the field +names — and there are still **two** spellings to keep in step, not three. + +Three things here have no wire partner, because no route publishes them: an +export report (the API returns the archive itself, not a description of it), a +thumbnail backfill, and a schema diff. They are gated only for encoding, at the +bottom of the parity test. Leaf encoding is explicit everywhere: UUIDs as strings, enums as ``.value``, paths as strings, and timestamps in **pydantic's** format — microseconds, ``Z`` @@ -43,20 +53,29 @@ from uuid import UUID from visionset.kernel.domain import ( + Annotation, AnnotationJob, AnnotationSchema, Asset, AssetProgress, Attribute, Batch, + BboxGeometry, + ClassCount, + ClassificationGeometry, Dataset, + DatasetStats, ExportResult, + Geometry, IngestFailure, IngestJob, LabelClass, + PolygonGeometry, Project, Release, ReleaseVerification, + SchemaChange, + SchemaDiff, Source, SplitRecipe, ThumbnailBackfill, @@ -130,6 +149,31 @@ def schema_version(value: AnnotationSchema) -> dict[str, Any]: } +def schema_change(value: SchemaChange) -> dict[str, Any]: + """One difference between two schema versions, and which kind it is.""" + return { + "kind": value.kind.value, + "label_class": value.label_class, + "attribute": value.attribute, + "detail": value.detail, + } + + +def schema_diff(value: SchemaDiff) -> dict[str, Any]: + """A proposed or actual schema change, classified. **Surface-defined**: no route reaches this. + + ``is_destructive`` and ``destructive_classes`` are domain ``@property`` + values materialized here, the way ``ReleaseVerification.ok`` is: a caller + deciding whether it needs ``allow_destructive`` must not have to re-derive + the answer from the ``changes`` list and get it subtly wrong. + """ + return { + "is_destructive": value.is_destructive, + "destructive_classes": sorted(value.destructive_classes), + "changes": [schema_change(c) for c in value.changes], + } + + # --- sources, ingest and assets ---------------------------------------------- @@ -194,8 +238,25 @@ def asset(value: Asset) -> dict[str, Any]: } +def batch_asset( + value: Asset, *, job_id: UUID | None, progress: AssetProgress | None +) -> dict[str, Any]: + """One asset seen from inside a batch: the asset, plus where the work stands. + + Widens :func:`asset` rather than replacing it, which is what the wire model + does by inheriting ``AssetOut`` — they are the same asset from a different + vantage point, and a field added to one belongs to both. Both extra fields + are null exactly while the batch is a draft, because a draft has no jobs. + """ + return { + **asset(value), + "job_id": None if job_id is None else str(job_id), + "progress": None if progress is None else progress.value, + } + + def thumbnail_backfill(value: ThumbnailBackfill) -> dict[str, Any]: - """A preview pass over a project. **CLI-defined**: no route reaches this.""" + """A preview pass over a project. **Surface-defined**: no route reaches this.""" return { "project_id": str(value.project_id), "examined": value.examined, @@ -248,6 +309,84 @@ def asset_progress(asset_id: UUID, progress: AssetProgress) -> dict[str, Any]: return {"asset_id": str(asset_id), "progress": progress.value} +# --- annotations ------------------------------------------------------------- + + +def geometry(value: Geometry) -> dict[str, Any]: + """One shape, tagged by ``type``. Coordinates are the asset's own pixels. + + Never normalized, at any surface — the domain's rule, and the one thing a + caller reading a scaled-down preview has to know. ``match`` on the concrete + class rather than on ``value.type``, so a variant added to the union without + a projection is a mypy error here instead of a ``KeyError`` at a caller. + """ + match value: + case BboxGeometry(): + return { + "type": value.type.value, + "x": value.x, + "y": value.y, + "width": value.width, + "height": value.height, + } + case PolygonGeometry(): + return {"type": value.type.value, "points": [list(p) for p in value.points]} + case ClassificationGeometry(): + return {"type": value.type.value} + + +def annotation(value: Annotation) -> dict[str, Any]: + """One label on one asset. ``schema_version`` is published on the way out only. + + A caller never sets it — the service stamps the batch's pinned version over + whatever it was handed — but reading it back is how a caller knows which + contract the label was judged against. + """ + return { + "id": str(value.id), + "asset_id": str(value.asset_id), + "label_class": value.label_class, + "schema_version": value.schema_version, + "geometry": geometry(value.geometry), + "attributes": dict(value.attributes), + "provenance": value.provenance, + "model_ref": value.model_ref, + "confidence": value.confidence, + } + + +# --- datasets ---------------------------------------------------------------- + + +def class_count(value: ClassCount) -> dict[str, Any]: + """How much of one class a dataset holds — both totals, deliberately. + + A thousand labels over a thousand images and the same thousand over ten are + the same ``annotations`` and a very different dataset. + """ + return { + "label_class": value.label_class, + "annotations": value.annotations, + "assets": value.assets, + } + + +def dataset_stats(value: DatasetStats) -> dict[str, Any]: + """What is in the trunk right now. Derived per call; a release freezes its own. + + ``classes`` rather than the domain's ``per_class``, matching the wire model: + a class the schema declares but nobody used is **absent**, so this is what + was counted and not what could be. + """ + return { + "dataset_id": str(value.dataset_id), + "asset_count": value.asset_count, + "annotated_asset_count": value.annotated_asset_count, + "annotation_count": value.annotation_count, + "classes": [class_count(c) for c in value.per_class], + } + + # --- releases, exports and formats ------------------------------------------- @@ -292,7 +431,7 @@ def export_format(value: Exporter) -> dict[str, Any]: def export_result(value: ExportResult) -> dict[str, Any]: - """What an export left on disk. **CLI-defined**: the API returns the archive. + """What an export left on disk. **Surface-defined**: the API returns the archive. ``directory`` is here where ``Asset.uri`` is not, and the difference is who chose it: this is the path the caller typed on ``--out``, so echoing it tells diff --git a/tests/cli/test_json_contract.py b/tests/cli/test_json_contract.py index 3bca737e..ecc6a340 100644 --- a/tests/cli/test_json_contract.py +++ b/tests/cli/test_json_contract.py @@ -1,9 +1,12 @@ -"""``--json`` and the REST API publish the same shape for the same concept. +"""``visionset.wire`` and the REST API publish the same shape for the same concept. -The two packages may not import each other — import-linter's independence -contract — so nothing enforces the agreement from inside ``src/``. A test can: -``tests/`` is outside the ``visionset`` package, so this module imports both -``visionset.cli._json`` and ``visionset.server.models`` and asserts, per pair: +``visionset.wire`` is what ``--json`` prints and what an MCP tool returns; the +server publishes the same concepts as pydantic models, because ``openapi.json`` +is generated from them and a dict cannot generate a schema. Two spellings, and +nothing inside ``src/`` can enforce the agreement — import-linter's independence +contract keeps the surfaces siblings. A test can: ``tests/`` is outside the +``visionset`` package, so this module imports both ``visionset.wire`` and +``visionset.server.models`` and asserts, per pair: 1. the projection's keys are exactly the wire model's fields; 2. the wire model *validates* the projection — which catches encoding drift a @@ -12,8 +15,8 @@ 3. the projection is JSON-serializable with no ``default=``, so a leaf somebody forgot to encode is a ``TypeError`` here rather than a silent ``str()``. -Two projections are deliberately ungated, and named at the bottom: the CLI -defines those shapes first because no route publishes them. +Three projections are deliberately ungated, and named at the bottom: a surface +defines those shapes first, because no route publishes them. """ from __future__ import annotations @@ -25,15 +28,22 @@ import pytest from pydantic import BaseModel from tests.fixtures.samples import ( + ANNOTATION, ASSET, BATCH, + BBOX, + CLASSIFICATION, COUNTS, + DATASET, + DATASET_STATS, EXPORT_RESULT, INGEST_FAILURE, INGEST_JOB, JOB, + POLYGON, PROJECT, RELEASE, + SCHEMA_DIFF, SCHEMA_VERSION, SOURCE, SPLIT, @@ -41,38 +51,56 @@ VERIFICATION, ) -from visionset.cli import _json +from visionset import wire from visionset.formats._dummy import DummyExporter +from visionset.kernel.domain import AssetProgress from visionset.server import models # One row per pair: a label, the projected payload, and the wire model it must # agree with. Built eagerly — every projection runs at import, so a leaf that # does not encode fails collection rather than one parametrized case. PAIRS: list[tuple[str, dict[str, Any], type[BaseModel]]] = [ - ("project", _json.project(PROJECT), models.ProjectOut), - ("schema_version", _json.schema_version(SCHEMA_VERSION), models.SchemaVersionOut), - ("label_class", _json.label_class(SCHEMA_VERSION.classes[0]), models.LabelClassBody), + ("project", wire.project(PROJECT), models.ProjectOut), + ("dataset", wire.dataset(DATASET), models.DatasetOut), + ("schema_version", wire.schema_version(SCHEMA_VERSION), models.SchemaVersionOut), + ("label_class", wire.label_class(SCHEMA_VERSION.classes[0]), models.LabelClassBody), ( "attribute", - _json.attribute(SCHEMA_VERSION.classes[0].attributes[0]), + wire.attribute(SCHEMA_VERSION.classes[0].attributes[0]), models.AttributeBody, ), - ("source", _json.source(SOURCE), models.SourceOut), - ("video_provenance", _json.video_provenance(SOURCE.require_video()), models.VideoProvenanceOut), - ("ingest_job", _json.ingest_job(INGEST_JOB), models.IngestJobOut), - ("ingest_failure", _json.ingest_failure(INGEST_FAILURE), models.IngestFailureOut), - ("asset", _json.asset(ASSET), models.AssetOut), - ("progress_counts", _json.progress_counts(COUNTS), models.ProgressCounts), - ("batch", _json.batch(BATCH, COUNTS), models.BatchOut), - ("job", _json.job(JOB, batch_id=BATCH.id), models.JobOut), - ("release", _json.release(RELEASE), models.ReleaseOut), - ("split_recipe", _json.split_recipe(SPLIT), models.SplitRecipeBody), + ("source", wire.source(SOURCE), models.SourceOut), + ("video_provenance", wire.video_provenance(SOURCE.require_video()), models.VideoProvenanceOut), + ("ingest_job", wire.ingest_job(INGEST_JOB), models.IngestJobOut), + ("ingest_failure", wire.ingest_failure(INGEST_FAILURE), models.IngestFailureOut), + ("asset", wire.asset(ASSET), models.AssetOut), + ( + "batch_asset", + wire.batch_asset(ASSET, job_id=JOB.id, progress=AssetProgress.ANNOTATED), + models.BatchAssetOut, + ), + ("progress_counts", wire.progress_counts(COUNTS), models.ProgressCounts), + ("batch", wire.batch(BATCH, COUNTS), models.BatchOut), + ("job", wire.job(JOB, batch_id=BATCH.id), models.JobOut), + ( + "asset_progress", + wire.asset_progress(ASSET.id, AssetProgress.SKIPPED), + models.AssetProgressOut, + ), + ("annotation", wire.annotation(ANNOTATION), models.AnnotationOut), + ("geometry_bbox", wire.geometry(BBOX), models.BboxBody), + ("geometry_polygon", wire.geometry(POLYGON), models.PolygonBody), + ("geometry_classification", wire.geometry(CLASSIFICATION), models.ClassificationBody), + ("dataset_stats", wire.dataset_stats(DATASET_STATS), models.DatasetStatsOut), + ("class_count", wire.class_count(DATASET_STATS.per_class[0]), models.ClassCountOut), + ("release", wire.release(RELEASE), models.ReleaseOut), + ("split_recipe", wire.split_recipe(SPLIT), models.SplitRecipeBody), ( "release_verification", - _json.release_verification(VERIFICATION), + wire.release_verification(VERIFICATION), models.ReleaseVerificationOut, ), - ("export_format", _json.export_format(DummyExporter()), models.FormatOut), + ("export_format", wire.export_format(DummyExporter()), models.FormatOut), ] IDS = [label for label, _, _ in PAIRS] @@ -107,7 +135,7 @@ def test_the_projection_serializes_with_no_default_encoder(payload: dict[str, An def test_a_listing_is_an_object_with_items_and_a_total() -> None: - assert _json.page([{"id": "a"}, {"id": "b"}]) == { + assert wire.page([{"id": "a"}, {"id": "b"}]) == { "items": [{"id": "a"}, {"id": "b"}], "total": 2, } @@ -116,7 +144,7 @@ def test_a_listing_is_an_object_with_items_and_a_total() -> None: def test_an_empty_listing_is_still_an_object() -> None: # Never a bare array, and never a 404's moral equivalent: an empty collection # is a collection. - assert _json.page([]) == {"items": [], "total": 0} + assert wire.page([]) == {"items": [], "total": 0} # --- the timestamp format the parity gate depends on ------------------------- @@ -127,19 +155,23 @@ def test_a_timestamp_keeps_its_microseconds_and_ends_in_z() -> None: # single shared helper would pass every key-set assertion above and fail the # round-trip one. when = datetime(2026, 7, 28, 12, 34, 56, 789012, tzinfo=UTC) - assert _json._moment(when) == "2026-07-28T12:34:56.789012Z" + assert wire._moment(when) == "2026-07-28T12:34:56.789012Z" -# --- the two shapes with no wire partner ------------------------------------- +# --- the three shapes with no wire partner ----------------------------------- @pytest.mark.parametrize( "payload", - [_json.export_result(EXPORT_RESULT), _json.thumbnail_backfill(THUMBNAIL_BACKFILL)], - ids=["export_result", "thumbnail_backfill"], + [ + wire.export_result(EXPORT_RESULT), + wire.thumbnail_backfill(THUMBNAIL_BACKFILL), + wire.schema_diff(SCHEMA_DIFF), + ], + ids=["export_result", "thumbnail_backfill", "schema_diff"], ) -def test_a_cli_defined_shape_still_serializes(payload: dict[str, Any]) -> None: - # No route publishes either, so there is nothing to be parity-gated against. - # What still has to hold is that every leaf is encoded — which is the failure - # these two would otherwise be free to have. +def test_a_surface_defined_shape_still_serializes(payload: dict[str, Any]) -> None: + # No route publishes any of the three, so there is nothing to be parity-gated + # against. What still has to hold is that every leaf is encoded — which is the + # failure they would otherwise be free to have. json.dumps(payload) diff --git a/tests/fixtures/samples.py b/tests/fixtures/samples.py index 314ecd3f..7d6d5338 100644 --- a/tests/fixtures/samples.py +++ b/tests/fixtures/samples.py @@ -3,8 +3,8 @@ The `tests/fixtures/media.py` precedent: a plain module of module-level values, no pytest import and no fixtures, so anything may reach for it. It exists for -`tests/cli/test_json_contract.py`, which compares the CLI's JSON projections -against the server's wire models field by field. +`tests/cli/test_json_contract.py`, which compares `visionset.wire`'s JSON +projections against the server's wire models field by field. **Every optional field is populated.** A sample carrying `None` where a nested model belongs would let the projection of that nested model go unchecked, which @@ -18,6 +18,7 @@ from uuid import uuid4 from visionset.kernel.domain import ( + Annotation, AnnotationJob, AnnotationJobState, AnnotationSchema, @@ -26,6 +27,12 @@ Attribute, Batch, BatchState, + BboxGeometry, + ChangeKind, + ClassCount, + ClassificationGeometry, + Dataset, + DatasetStats, ExportResult, GeometryType, ImageFormat, @@ -34,9 +41,12 @@ IngestJob, IngestState, LabelClass, + PolygonGeometry, Project, Release, ReleaseVerification, + SchemaChange, + SchemaDiff, Source, SourceKind, SplitRecipe, @@ -73,6 +83,27 @@ ), ) +DATASET = Dataset( + id=uuid4(), project_id=PROJECT.id, name="road-signs", description="a sample project" +) + +SCHEMA_DIFF = SchemaDiff( + changes=( + SchemaChange( + kind=ChangeKind.ADDITIVE, + label_class="pedestrian", + attribute=None, + detail="class added", + ), + SchemaChange( + kind=ChangeKind.DESTRUCTIVE, + label_class="sign", + attribute="condition", + detail="attribute removed", + ), + ), +) + SOURCE = Source( project_id=PROJECT.id, kind=SourceKind.VIDEO, @@ -137,6 +168,34 @@ progress=dict.fromkeys(BATCH.asset_ids, AssetProgress.UNANNOTATED), ) +BBOX = BboxGeometry(x=1.5, y=2.5, width=30.0, height=40.0) +POLYGON = PolygonGeometry(points=[(0.0, 0.0), (10.0, 0.0), (10.0, 10.0)]) +CLASSIFICATION = ClassificationGeometry() + +# Every geometry variant gets its own sample rather than one standing for the +# union: they are three components on the wire, and a projection that dropped +# `points` would still round-trip through the bbox model. +GEOMETRIES = (BBOX, POLYGON, CLASSIFICATION) + +ANNOTATION = Annotation( + asset_id=ASSET.id, + label_class="sign", + schema_version=3, + geometry=BBOX, + attributes={"condition": "faded"}, + provenance="model", + model_ref="yolo-v8n@1", + confidence=0.87, +) + +DATASET_STATS = DatasetStats( + dataset_id=DATASET.id, + asset_count=2, + annotated_asset_count=1, + annotation_count=5, + per_class=(ClassCount(label_class="sign", annotations=5, assets=1),), +) + SPLIT = SplitRecipe(train=0.7, val=0.15, test=0.15, seed=42) RELEASE = Release( diff --git a/uv.lock b/uv.lock index 7f98998a..ee81b51e 100644 --- a/uv.lock +++ b/uv.lock @@ -1247,6 +1247,7 @@ dependencies = [ [package.dev-dependencies] dev = [ + { name = "anyio" }, { name = "httpx" }, { name = "import-linter" }, { name = "mypy" }, @@ -1268,6 +1269,7 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ + { name = "anyio", specifier = ">=4.5" }, { name = "httpx", specifier = ">=0.27" }, { name = "import-linter", specifier = ">=2.0" }, { name = "mypy", specifier = ">=1.10" }, From 788e043644fb918398ad7587c8c3226de7c7b657 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Tue, 28 Jul 2026 21:45:05 -0700 Subject: [PATCH 2/3] =?UTF-8?q?feat(mcp):=20real=20tools=20over=20the=20SD?= =?UTF-8?q?K=20=E2=80=94=20thirty-three=20of=20them,=20and=20an=20agent=20?= =?UTF-8?q?that=20can=20see?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the thirteen stubs with the MCP tool sweep #35 asks for: fifty parity candidates evaluated one by one, thirty ship, twenty fold or drop, and three are new or merged. `visionset mcp` starts it. `get_asset_image` is the tool the milestone was for. Without it an agent can drive the whole workflow and never see what it is annotating. It returns the cached preview plus four numbers, because a preview is capped at 256 on its long edge while geometry is always in the asset's native pixels: `width`/`height` are the frame to write in, `image_width`/`image_height` are what was sent, and `scale` is the factor between them. An agent that skipped the multiplication would produce annotations that are individually plausible and uniformly wrong, and nothing downstream could detect it. `preview_schema_change` gives `SchemaService.preview` its first caller since #6. Conventions: the workspace is opened per call and named only by the environment; domain models go straight into tool signatures, so their docstrings reach the agent as `$defs` and their validators refuse malformed input; refusals are one envelope carrying the kernel's own sentence plus `retry_with`, which answers the only question a machine-readable code was needed for — `allow_destructive` retries a narrowing schema change and nothing retries an orphaning one. Registration is one table in `main.py`, which is also where `guarded`, `inspect.cleandoc` and the read/write hints are applied, so none of the three can be forgotten by the next tool. No migration: FORMAT_VERSION stays 11. `openapi.json` and the generated client are both byte-identical — MCP touches no route. 1896 tests, up from 1670. --- src/visionset/cli/main.py | 8 +- src/visionset/cli/mcp.py | 77 +++++++++ src/visionset/mcp/_errors.py | 170 ++++++++++++++++++++ src/visionset/mcp/_resolve.py | 110 +++++++++++++ src/visionset/mcp/_workspace.py | 55 +++++++ src/visionset/mcp/annotations.py | 213 +++++++++++++++++++++++++ src/visionset/mcp/assets.py | 175 +++++++++++++++++++++ src/visionset/mcp/batches.py | 200 ++++++++++++++++++++++++ src/visionset/mcp/datasets.py | 45 ++++++ src/visionset/mcp/formats.py | 30 ++++ src/visionset/mcp/jobs.py | 137 ++++++++++++++++ src/visionset/mcp/main.py | 240 ++++++++++++++-------------- src/visionset/mcp/projects.py | 111 +++++++++++++ src/visionset/mcp/releases.py | 173 ++++++++++++++++++++ src/visionset/mcp/schemas.py | 152 ++++++++++++++++++ src/visionset/mcp/sources.py | 155 ++++++++++++++++++ tests/cli/test_mcp_command.py | 182 +++++++++++++++++++++ tests/mcp/_flow.py | 156 ++++++++++++++++++ tests/mcp/test_agent_walk.py | 179 +++++++++++++++++++++ tests/mcp/test_annotation_tools.py | 219 ++++++++++++++++++++++++++ tests/mcp/test_asset_tools.py | 177 +++++++++++++++++++++ tests/mcp/test_batch_tools.py | 180 +++++++++++++++++++++ tests/mcp/test_ingest_tools.py | 172 ++++++++++++++++++++ tests/mcp/test_job_tools.py | 152 ++++++++++++++++++ tests/mcp/test_project_tools.py | 133 ++++++++++++++++ tests/mcp/test_registration.py | 176 +++++++++++++++++++++ tests/mcp/test_release_tools.py | 243 +++++++++++++++++++++++++++++ tests/mcp/test_schema_tools.py | 160 +++++++++++++++++++ tests/mcp/test_tool_errors.py | 176 +++++++++++++++++++++ 29 files changed, 4229 insertions(+), 127 deletions(-) create mode 100644 src/visionset/cli/mcp.py create mode 100644 src/visionset/mcp/_errors.py create mode 100644 src/visionset/mcp/_resolve.py create mode 100644 src/visionset/mcp/_workspace.py create mode 100644 src/visionset/mcp/annotations.py create mode 100644 src/visionset/mcp/assets.py create mode 100644 src/visionset/mcp/batches.py create mode 100644 src/visionset/mcp/datasets.py create mode 100644 src/visionset/mcp/formats.py create mode 100644 src/visionset/mcp/jobs.py create mode 100644 src/visionset/mcp/projects.py create mode 100644 src/visionset/mcp/releases.py create mode 100644 src/visionset/mcp/schemas.py create mode 100644 src/visionset/mcp/sources.py create mode 100644 tests/cli/test_mcp_command.py create mode 100644 tests/mcp/_flow.py create mode 100644 tests/mcp/test_agent_walk.py create mode 100644 tests/mcp/test_annotation_tools.py create mode 100644 tests/mcp/test_asset_tools.py create mode 100644 tests/mcp/test_batch_tools.py create mode 100644 tests/mcp/test_ingest_tools.py create mode 100644 tests/mcp/test_job_tools.py create mode 100644 tests/mcp/test_project_tools.py create mode 100644 tests/mcp/test_registration.py create mode 100644 tests/mcp/test_release_tools.py create mode 100644 tests/mcp/test_schema_tools.py create mode 100644 tests/mcp/test_tool_errors.py diff --git a/src/visionset/cli/main.py b/src/visionset/cli/main.py index 05615273..0d0f6ae0 100644 --- a/src/visionset/cli/main.py +++ b/src/visionset/cli/main.py @@ -13,6 +13,7 @@ from visionset.cli.ingest import backfill_thumbnails, ingest from visionset.cli.init import init from visionset.cli.jobs import job_app +from visionset.cli.mcp import mcp from visionset.cli.projects import project_app from visionset.cli.releases import release_app from visionset.cli.schemas import schema_app @@ -51,6 +52,7 @@ app.command("backfill-thumbnails")(backfill_thumbnails) app.add_typer(token_app, name="token") app.command()(ui) +app.command()(mcp) def _version_callback(value: bool) -> None: @@ -72,9 +74,3 @@ def main( ] = False, ) -> None: """Robomous VisionSet CLI.""" - - -@app.command() -def mcp() -> None: - """Start the MCP server on stdio (stub).""" - typer.echo("server would start here") diff --git a/src/visionset/cli/mcp.py b/src/visionset/cli/mcp.py new file mode 100644 index 00000000..91044cf2 --- /dev/null +++ b/src/visionset/cli/mcp.py @@ -0,0 +1,77 @@ +# usage: from visionset.cli.mcp import mcp +"""``visionset mcp`` — the front door for an agent: one command, thirty-three tools. + +``ui.py``'s shape exactly, with a subprocess where that one has uvicorn, and the +same three decisions behind it. + +**The server is named, never imported.** import-linter forbids ``visionset.cli`` +importing ``visionset.mcp``, so the target is spelled as a module for the +interpreter to find. ``ui`` gets to hand uvicorn an import string; there is no +equivalent here, so this spawns ``python -m visionset.mcp.main`` and lets it +inherit stdin and stdout — which is the whole point, because those two streams +*are* the MCP transport. Nothing is captured, nothing is piped, and this process +does nothing but wait and pass on the exit code. + +**Configuration travels by environment, because there is no other channel.** The +child takes no arguments, so the resolved workspace reaches it as +``VISIONSET_WORKSPACE``. This command applies the *full* four-branch precedence — +including the upward walk — and then **states** the answer, so the child's own +``resolve_workspace_root`` stops at branch 2 and the two cannot disagree. + +**The workspace is opened and closed before the child exists.** Not a check but a +real ``open``: it runs the migration, so ``NotAWorkspace``, ``WorkspaceCorrupt`` +and ``WorkspaceFormatTooNew`` land at a terminal as one sentence and exit 1 rather +than inside the agent's first tool call, where the answer is a JSON envelope +nobody is watching. Closing again matters for the same reason it does in ``ui``: +an uncheckpointed SQLite leaves ``visionset.db-wal`` behind, and the child is +about to open the file for itself. + +**Nothing is printed on stdout, ever.** Stdout belongs to the JSON-RPC stream, so +a single stray line would corrupt the protocol before the first message. The +banner goes to stderr, which is where an MCP client collects a server's logs. + +**A client normally spawns this itself** rather than a person running it, with +``VISIONSET_WORKSPACE`` set in the server entry's own ``env`` — see +``docs/mcp.md``. The flag is what makes the command usable by hand and testable. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from typing import Final + +import typer + +from visionset import __version__ +from visionset.cli._workspace import WorkspaceOption, opened_workspace +from visionset.kernel.services import WORKSPACE_ENV_VAR + +SERVER_MODULE: Final = "visionset.mcp.main" +"""What the child interpreter is told to run. See the module docstring for why.""" + + +def mcp(workspace: WorkspaceOption = None) -> None: + """Start the MCP server on stdio, serving this workspace to an agent. + + Speaks JSON-RPC on stdin and stdout, so run it from an MCP client rather than + by hand. Every tool operates on the workspace resolved here, and no token is + involved: an agent reaching this server is already inside the sandbox the + workspace defines. + """ + with opened_workspace(workspace) as service: + root = service.root + os.environ[WORKSPACE_ENV_VAR] = str(root) + + typer.secho(f"VisionSet {__version__} — MCP server on stdio", err=True, bold=True) + typer.echo(f" workspace {root}", err=True) + typer.echo("Press Ctrl+C to stop.", err=True) + + # `sys.executable`, not "python": a workspace's virtual environment is + # frequently not what `python` resolves to on PATH, and the child has to be + # the interpreter this command is already running under or it will not find + # `visionset` at all. + completed = subprocess.run([sys.executable, "-m", SERVER_MODULE], check=False) # noqa: S603 + if completed.returncode: + raise typer.Exit(code=completed.returncode) diff --git a/src/visionset/mcp/_errors.py b/src/visionset/mcp/_errors.py new file mode 100644 index 00000000..e8f79bb3 --- /dev/null +++ b/src/visionset/mcp/_errors.py @@ -0,0 +1,170 @@ +# usage: from visionset.mcp._errors import guarded +"""What a kernel refusal looks like to an agent: a sentence, and what to do next. + +The third spelling of a contract ``server/errors.py`` keeps over HTTP and +``cli/_errors.py`` keeps at a terminal, and it sits between them on purpose. A +REST client branches on a machine-readable ``code`` because it is a program; a +shell branches on zero versus non-zero because a person reads the sentence. An +agent is both — it reads, and it decides — so it gets the kernel's own sentence +**and** one machine-readable field: + +.. code-block:: json + + {"error": {"message": "...", "retry_with": "allow_destructive", + "hint": null, "index": null}} + +**There is deliberately no ``code``.** The codes live in ``server/errors.py``'s +``ERROR_RULES``, which this package may not import, and the standing rule from +#31 forbids deriving one from a class name — a public contract keyed to a Python +identifier breaks silently on a refactor and passes every test. What a code was +actually needed for here is one question, "may I retry this, and with what?", and +:data:`RETRY_WITH` answers it directly. ``DESTRUCTIVE_SCHEMA_CHANGE`` is +retryable with a flag and ``SCHEMA_CHANGE_WOULD_ORPHAN`` is not; publishing the +flag rather than the code is what keeps an agent out of the retry loop +``SchemaChangeWouldOrphan``'s own docstring warns about. If a caller ever needs +the full table, ``ERROR_RULES``' code column gets promoted beside +``visionset.wire`` — it is not re-spelled here. + +**Every tool is wrapped once, in ``main.py``'s registration table**, rather than +each body decorating itself. One spelling that cannot be forgotten, and +``test_registration.py`` asserts every registered tool went through it. Letting +an exception escape instead is not neutral: FastMCP catches it, prepends +``"Error executing tool X: "`` and ships ``str(exc)`` to the client anyway — the +same disclosure with none of the structure. + +**Only ``VisionSetError`` is caught**, the CLI's rule. A ``FileNotFoundError``, a +bare ``ValueError`` from a non-positive rate, or a pydantic ``ValidationError`` +from a geometry built out of tool arguments are not refusals this surface +understands, and each is guarded at the one call site that can raise it — +:func:`refused` is how a tool body reports one in the same envelope without +pretending it came from the kernel. +""" + +from __future__ import annotations + +import functools +import inspect +import json +from collections.abc import Callable +from typing import Any, Final + +from mcp.types import CallToolResult, TextContent + +from visionset.kernel import ( + ConfirmationRequired, + DestructiveSchemaChange, + LossyExportNotConsented, + NotAWorkspace, + ThumbnailNotCached, + VisionSetError, +) +from visionset.kernel.services import WORKSPACE_ENV_VAR + +RETRY_WITH: Final[dict[type[BaseException], str]] = { + # The three gate words, never merged into one. Each guards a different thing: + # `confirm` destroying data, `allow_destructive` narrowing a contract, + # `allow_lossy` emitting an incomplete copy of something that stays intact. + ConfirmationRequired: "confirm", + DestructiveSchemaChange: "allow_destructive", + LossyExportNotConsented: "allow_lossy", +} +"""The parameter that turns this refusal into the same call, succeeding. + +Walked by MRO, the way ``server/errors.py`` walks ``ERROR_RULES``. Sparse on +purpose: **most refusals are not retryable at all**, and +``SchemaChangeWouldOrphan`` is deliberately absent rather than mapped — it is not +a subclass of ``DestructiveSchemaChange`` precisely so that no flag appears to +override it. +""" + +_HINTS: Final[dict[type[BaseException], str]] = { + # A remedy an *agent* can act on, which is not the same set the CLI has. The + # kernel's own sentence for a missing workspace ends in "use + # WorkspaceService.init", a Python call; an agent cannot make one, and it + # cannot set an environment variable either — so the hint names the thing + # whoever configured the client has to fix. + NotAWorkspace: ( + f"No workspace is configured. Whoever runs this MCP server must set " + f"{WORKSPACE_ENV_VAR} in its environment, or start it with " + f"`visionset mcp --workspace `." + ), + ThumbnailNotCached: ( + "Call `backfill_thumbnails` for this project to render the missing " + "previews, or ask for `full=true` to read the original bytes." + ), +} +"""A next step this surface can add under the kernel's sentence. + +Sparse for the reason the CLI's is: a hint that restates the message is noise, +and most kernel sentences already carry their own remedy. +""" + + +def _walk(table: dict[type[BaseException], str], exc: BaseException) -> str | None: + for cls in type(exc).__mro__: + found = table.get(cls) + if found is not None: + return found + return None + + +def refused(message: str, *, hint: str | None = None) -> dict[str, Any]: + """The same envelope, for a refusal this surface made rather than the kernel. + + Used where a kernel call would otherwise raise something outside the + ``VisionSetError`` tree — a missing path, a non-positive rate — so the shape + a caller parses does not depend on which layer said no. + """ + return {"error": {"message": message, "retry_with": None, "hint": hint, "index": None}} + + +def _envelope(exc: VisionSetError) -> dict[str, Any]: + return { + "error": { + "message": str(exc), + "retry_with": _walk(RETRY_WITH, exc), + "hint": _walk(_HINTS, exc), + # Which item of a sequence the caller passed is at fault. Set by + # `AnnotationService`'s `_blaming` and unrecoverable at the boundary: + # the write is all-or-nothing, so nothing landed and there is no + # partial result to count from. + "index": exc.index, + } + } + + +def guarded[**P, T](fn: Callable[P, T]) -> Callable[P, T | dict[str, Any]]: + """Turn any kernel refusal raised by ``fn`` into the error envelope. + + ``functools.wraps`` is load-bearing rather than cosmetic: FastMCP builds a + tool's ``inputSchema`` from ``inspect.signature(fn, eval_str=True)`` and its + description from ``fn.__doc__``, and the signature call follows ``__wrapped__`` + back to the annotations' own module. Without it every tool would take + ``*args, **kwargs`` and document nothing. + + **A tool that returns ``CallToolResult`` gets its refusal wrapped in one too**, + and that is not a detail. FastMCP puts a returned ``dict`` into + ``structuredContent`` only when the declared return type is a mapping; from a + tool declared ``-> CallToolResult`` the same dict comes back as JSON text with + ``structuredContent`` **null**. ``get_asset_image`` is the one such tool, and + without this its refusals would be the only answers in the whole surface a + client had to parse out of a text block. The check is done once, here, rather + than in that tool's body, so it cannot be forgotten by the next one. + """ + returns_result = inspect.signature(fn, eval_str=True).return_annotation is CallToolResult + + @functools.wraps(fn) + def guard(*args: P.args, **kwargs: P.kwargs) -> T | dict[str, Any]: + try: + return fn(*args, **kwargs) + except VisionSetError as exc: + envelope = _envelope(exc) + if not returns_result: + return envelope + rendered = CallToolResult( + content=[TextContent(type="text", text=json.dumps(envelope, indent=2))], + structuredContent=envelope, + ) + return rendered # type: ignore[return-value] + + return guard diff --git a/src/visionset/mcp/_resolve.py b/src/visionset/mcp/_resolve.py new file mode 100644 index 00000000..9e2a6cc3 --- /dev/null +++ b/src/visionset/mcp/_resolve.py @@ -0,0 +1,110 @@ +# usage: from visionset.mcp._resolve import resolve_project, resolve_release +"""Turning what an agent said into the thing it meant. + +The same two resources the CLI can name, for the same reason and with the same +two opposite rules: + +- a **project**, whose name is unique per workspace **case-insensitively**; +- a **release**, whose tag is unique per dataset and **case-sensitive**. + +Neither comparison is spelled here. ``ProjectService.get_by_name`` and +``ReleaseService.get_by_tag`` are kernel reads precisely so the rule lives beside +the index that enforces it — a surface re-deriving one from prose is a second +spelling free to drift, and it would eventually pick the wrong one. What this +module owns is only the dispatch: a well-formed UUID is an id, anything else is a +name. + +Name resolution matters more here than at a terminal, not less. A person reads an +id off the previous command's output; an agent carries it in a context window and +will paraphrase one given the chance. ``"road-signs"`` survives that and +``9f2c…`` does not. + +**Batches, jobs and assets are addressed by id and nothing else.** A batch has a +name but it is not unique — an ingest names one after its source, and re-ingesting +the same folder makes a second batch with a name just as good — so resolving one +by name would have to pick, and picking is worse than refusing. Their ids come +back from the tool that created them. + +Unlike the CLI, a malformed UUID cannot arrive as a usage error at exit 2: an id +parameter is typed ``str`` on the wire either way. Each tool parses one through +:func:`identifier`, so a value that could not have named anything is refused in +the ordinary envelope rather than reaching the kernel as a puzzling ``*NotFound``. +""" + +from __future__ import annotations + +from typing import Annotated +from uuid import UUID + +from pydantic import Field + +from visionset.kernel.domain import Project, Release +from visionset.kernel.errors import VisionSetError +from visionset.kernel.services import ProjectService, ReleaseService, WorkspaceService + +ProjectRef = Annotated[ + str, + Field(description="The project, by name (case-insensitive) or by id."), +] +"""``project``, for a tool scoped to one project. + +Module-level so that ``inspect.signature(fn, eval_str=True)`` resolves it in the +importing module's globals under ``from __future__ import annotations``; an alias +built inside a function body would not resolve, and FastMCP would refuse the tool +at registration. +""" + + +class MalformedIdentifier(VisionSetError): + """A parameter that has to be an id is not one. + + A ``VisionSetError`` subclass rather than a bare ``ValueError`` so that + ``guarded`` renders it as the ordinary envelope. It is deliberately **not** in + ``kernel/errors.py``: the kernel takes ``UUID`` objects and cannot be handed a + malformed one, so this is a fact about a surface whose arguments arrive as + JSON strings, which is exactly the same call the API makes when it answers 422 + rather than 404 to an unparseable path segment. + """ + + +def identifier(value: str, *, what: str) -> UUID: + """The UUID that string spells, or say it is not one. + + Raises: + MalformedIdentifier: the value is not a well-formed UUID. + """ + try: + return UUID(value) + except ValueError: + raise MalformedIdentifier( + f"{what} must be a UUID, and {value!r} is not one; " + f"ids come back from the tool that created the thing" + ) from None + + +def resolve_project(workspace: WorkspaceService, reference: str) -> Project: + """The project that reference names, by id if it parses as one, else by name. + + A project whose *name* is a well-formed UUID string is unreachable by name. + That is harmless: the same string reaches it as an id, and it is the same + string either way. + """ + projects = ProjectService(workspace) + try: + project_id = UUID(reference) + except ValueError: + return projects.get_by_name(reference) + return projects.get(project_id) + + +def resolve_release(workspace: WorkspaceService, reference: str, tag: str) -> Release: + """The release under that tag, in the dataset of the project reference names. + + Two lookups rather than one, because a release tag is unique per *dataset* + and a dataset is reached through its project. The intermediate read is not + waste: it is what makes an unknown project say so, instead of reporting a + perfectly good tag as missing. + """ + project = resolve_project(workspace, reference) + dataset = ProjectService(workspace).get_dataset(project.id) + return ReleaseService(workspace).get_by_tag(dataset.id, tag) diff --git a/src/visionset/mcp/_workspace.py b/src/visionset/mcp/_workspace.py new file mode 100644 index 00000000..ebebe097 --- /dev/null +++ b/src/visionset/mcp/_workspace.py @@ -0,0 +1,55 @@ +# usage: from visionset.mcp._workspace import opened_workspace +"""Which workspace the tools operate on, and how long it stays open. + +The rule is the kernel's (``resolve_workspace_root``) and this module is the MCP +surface's half of it — which is nothing but the ``with`` block, because there is +no flag to feed it. + +**No tool takes a ``workspace`` parameter.** Threading one through thirty-three +tools would put a path an agent has no way to know into every call, and an agent +that guessed wrong would be writing into a workspace nobody pointed it at. The +answer comes from the environment instead: an MCP client names the server in its +own configuration and sets ``VISIONSET_WORKSPACE`` there, which is what +``visionset mcp --workspace`` does on its behalf. That lands every server on +precedence branch 2, or on branch 3's upward walk from whatever directory the +client happened to spawn it in. + +**The workspace is opened per call and closed again**, unlike the HTTP server, +which builds one handle in ``create_app()`` and keeps it. Three reasons, and the +first is the one that decided it: + +1. There is no module-level mutable state, so every tool is testable in isolation + with nothing but ``monkeypatch.setenv`` — where a process-lifetime handle + would have to be torn down between tests and would leak a workspace into the + next module when a test forgot. +2. SQLite has one writer. A stdio server that held the file between calls would + keep ``visionset ui`` and a second agent out of a workspace nobody is using. +3. ``close()`` checkpoints the WAL, so a client that disappears mid-session + leaves no ``visionset.db-wal`` behind. + +What it costs is a file open and a migration check per call, which is the same +work ``visionset`` does per command. +""" + +from __future__ import annotations + +from collections.abc import Iterator +from contextlib import contextmanager + +from visionset.kernel.services import WorkspaceService, resolve_workspace_root + + +@contextmanager +def opened_workspace() -> Iterator[WorkspaceService]: + """The configured workspace, open for the length of the body. + + Closes on the way out, including when the body raised. Refusals from the + open itself — ``NotAWorkspace``, ``WorkspaceCorrupt``, + ``WorkspaceFormatTooNew`` — travel to the caller as the ordinary error + envelope, because ``guarded`` wraps the whole tool and not just its middle. + """ + workspace = WorkspaceService.open(resolve_workspace_root()) + try: + yield workspace + finally: + workspace.close() diff --git a/src/visionset/mcp/annotations.py b/src/visionset/mcp/annotations.py new file mode 100644 index 00000000..cf713b71 --- /dev/null +++ b/src/visionset/mcp/annotations.py @@ -0,0 +1,213 @@ +# usage: from visionset.mcp import annotations +"""Annotation tools: the writes that make an agent an annotator rather than an operator. + +**Two input models, and they are the only ones in this package.** Everywhere else +a domain model goes straight into the signature; ``Annotation`` cannot, because +it requires ``schema_version`` and the service *overwrites* whatever it is given +with the version its batch pinned. A required field whose value is discarded is a +lie in the input schema, and an agent that reasoned about which version to send +would be reasoning about nothing. So ``AnnotationInput`` omits it, exactly as +``AnnotationCreate`` does on the wire, and the geometry is still the domain's own +union. + +All three writes are **one transaction and all-or-nothing**: a batch of ten +annotations with one bad geometry writes none of them. When that happens the +refusal carries ``index``, the position in the list you sent — which is +recoverable nowhere else, because nothing landed for the caller to count from. + +``delete_annotations`` has **no ``confirm``**, and that is deliberate rather than +an oversight. Deleting a box is the annotator edit loop, not a destructive +operation; the guard is the batch gate, which refuses every write once a batch is +no longer open. It is one of exactly two methods in the whole kernel exempted +from ``confirm``, and the exemption is written down in the error's own docstring. +""" + +from __future__ import annotations + +from typing import Annotated, Any +from uuid import UUID, uuid4 + +from pydantic import BaseModel, ConfigDict, Field + +from visionset import wire +from visionset.kernel.domain import Annotation, AttributeValue, Geometry, Provenance +from visionset.kernel.services import AnnotationService +from visionset.mcp._resolve import identifier +from visionset.mcp._workspace import opened_workspace + +JobRef = Annotated[str, Field(description="The annotation job the write belongs to, by id.")] +"""Module-level for the ``inspect.signature`` reason.""" + + +class AnnotationInput(BaseModel): + """One label to write. ``schema_version`` is absent: the batch's pin is stamped in.""" + + model_config = ConfigDict(extra="forbid") + + asset_id: UUID = Field(description="The asset this label is on. Must belong to the job.") + label_class: str = Field(description="A class name the batch's pinned schema declares.") + geometry: Geometry = Field( + description=( + "The shape, in the asset's own pixel coordinates — never normalized. " + "Its 'type' field is required and selects the variant." + ) + ) + attributes: dict[str, AttributeValue] = Field( + default_factory=dict, + description="Values keyed by the attribute names the class declares.", + ) + provenance: Provenance = Field( + description="Who produced it. Use 'model' for anything you inferred." + ) + model_ref: str | None = Field( + default=None, + description="Which model produced it. Required when provenance is 'model'.", + ) + confidence: float | None = Field( + default=None, ge=0.0, le=1.0, description="Optional confidence between 0 and 1." + ) + + def to_domain(self) -> Annotation: + """The domain model, with a placeholder version the service replaces.""" + return Annotation( + asset_id=self.asset_id, + label_class=self.label_class, + # Any value ≥ 1 does; `AnnotationService` stamps the batch's pin over + # it before anything is stored. `1` is what the REST body passes for + # the same reason. + schema_version=1, + geometry=self.geometry, + attributes=self.attributes, + provenance=self.provenance, + model_ref=self.model_ref, + confidence=self.confidence, + ) + + +class AnnotationEdit(BaseModel): + """One label to replace, addressed by id. ``asset_id`` is absent: the stored one wins.""" + + model_config = ConfigDict(extra="forbid") + + id: UUID = Field(description="The annotation to replace, by id.") + label_class: str = Field(description="A class name the batch's pinned schema declares.") + geometry: Geometry = Field( + description="The replacement shape, in the asset's own pixel coordinates." + ) + attributes: dict[str, AttributeValue] = Field(default_factory=dict) + provenance: Provenance = Field(description="Who produced this version of it.") + model_ref: str | None = Field(default=None) + confidence: float | None = Field(default=None, ge=0.0, le=1.0) + + def to_domain(self) -> Annotation: + """The domain model. Both the version and the asset are replaced by the service.""" + return Annotation( + id=self.id, + # The service overwrites this with the *stored* annotation's asset, + # which is the rule that stops an update moving a label to another + # image. A throwaway id is the honest way to say it is not an input. + asset_id=uuid4(), + label_class=self.label_class, + schema_version=1, + geometry=self.geometry, + attributes=self.attributes, + provenance=self.provenance, + model_ref=self.model_ref, + confidence=self.confidence, + ) + + +def list_asset_annotations( + job_id: JobRef, + asset_id: Annotated[str, Field(description="The asset within that job, by id.")], +) -> dict[str, Any]: + """List the annotations already written on one asset of a job. + + Read this before editing: `update_annotations` and `delete_annotations` both + address annotations by `id`, and this is where the ids come from. An asset + with nothing on it yet is an empty list, not an error. + """ + with opened_workspace() as workspace: + found = AnnotationService(workspace).for_asset( + identifier(job_id, what="job_id"), identifier(asset_id, what="asset_id") + ) + return wire.page([wire.annotation(a) for a in found]) + + +def add_annotations( + job_id: JobRef, + annotations: Annotated[ + list[AnnotationInput], + Field(description="The labels to write. All are written, or none are."), + ], +) -> dict[str, Any]: + """Write new annotations into a job. All succeed together or none are written. + + Each is judged against the schema version the job's batch pinned: the class + must be declared there, the geometry must be the one that class is bound to, + every required attribute must be present, and no attribute the class does not + declare is allowed. Coordinates are the asset's own pixels — if you read the + image through `get_asset_image` at preview size, multiply by the `scale` it + returned before writing. + + Set `provenance` to `model` and give `model_ref` for anything you inferred; + that is what lets a human reviewer tell your work from theirs later. + + Writing an annotation moves its asset to `annotated` on its own. Refuses if + the job's batch is not `in_annotation`, and if any single item is bad the + refusal names its position in `index` — nothing was written, so that position + is the only thing that identifies it. + """ + with opened_workspace() as workspace: + written = AnnotationService(workspace).add( + identifier(job_id, what="job_id"), [a.to_domain() for a in annotations] + ) + return wire.page([wire.annotation(a) for a in written]) + + +def update_annotations( + job_id: JobRef, + annotations: Annotated[ + list[AnnotationEdit], + Field(description="The replacements, each addressed by id. All or none."), + ], +) -> dict[str, Any]: + """Replace existing annotations wholesale. All succeed together or none are written. + + A whole-value replace, not a patch: what you send is what the annotation + becomes, so send every field you want to keep. The asset an annotation is on + cannot be changed — the stored one always wins — and the schema version is + re-stamped from the batch's pin. + + Same validation and the same all-or-nothing rule as `add_annotations`, with + `index` naming the offending position. + """ + with opened_workspace() as workspace: + written = AnnotationService(workspace).update( + identifier(job_id, what="job_id"), [a.to_domain() for a in annotations] + ) + return wire.page([wire.annotation(a) for a in written]) + + +def delete_annotations( + job_id: JobRef, + annotation_ids: Annotated[ + list[str], Field(description="The annotations to remove, by id. All or none.") + ], +) -> dict[str, Any]: + """Remove annotations from a job. All succeed together or none are removed. + + No confirmation is required, unlike `delete_project`: deleting a label is the + ordinary edit loop, and the guard is that a batch which is no longer + `in_annotation` refuses every write. Removing every annotation from an asset + moves it back to `unannotated`. + + A repeated id counts once, and the refusal for an unknown one blames the + position you gave it rather than a deduplicated offset. + """ + with opened_workspace() as workspace: + removed = AnnotationService(workspace).delete( + identifier(job_id, what="job_id"), + [identifier(a, what="annotation_ids") for a in annotation_ids], + ) + return {"deleted": removed} diff --git a/src/visionset/mcp/assets.py b/src/visionset/mcp/assets.py new file mode 100644 index 00000000..447cdb74 --- /dev/null +++ b/src/visionset/mcp/assets.py @@ -0,0 +1,175 @@ +# usage: from visionset.mcp import assets +"""``get_asset_image`` — the tool that makes an agent an annotator, not an operator. + +Everything else in this package moves rows around. Without this one an agent can +drive the entire workflow and never see what it is annotating, which guts the +whole point: ``add_annotations`` with ``provenance='model'`` only means something +if the model looked. + +**The coordinate-frame trap, and why the answer carries four numbers.** A preview +is capped at ``DEFAULT_THUMBNAIL_MAX_EDGE`` on its long side, while annotation +geometry is *always* in the asset's native pixels and is never normalized. An +agent that reads a 256-pixel preview and writes boxes in preview pixels produces +annotations that are silently wrong — wrong in a way nothing downstream can +detect, because the numbers are individually plausible. So the structured half +publishes the asset's own ``width``/``height`` (the frame to write in), the +``image_width``/``image_height`` actually sent, and the ``scale`` between them, +and the description says which to multiply by. Publishing only one pair would +have been smaller and would have made the mistake invisible. + +**The preview's dimensions are measured, never derived.** The port pins a maximum +*edge*, not a size: a small image is never enlarged, and which edge is capped +depends on the aspect ratio. Computing them arithmetically would be a guess, and +a wrong ``scale`` is the one error this tool exists to prevent — so they come off +the encoded bytes, through ``ImageProcessor.probe``. Using the port rather than +importing Pillow here is what keeps a decoder out of a delivery module. + +**It folds ``get_asset``.** The structured half already carries every field the +row publishes, so a separate metadata tool would be a second round trip for +something this one has to send anyway. + +**Bare ``CallToolResult``, not ``Annotated[CallToolResult, Model]``.** The +annotated form declares an output schema and validates ``structuredContent`` +against it — which would reject the error envelope on the refusal path, since a +refusal cannot also be a valid image result. Verified against mcp 1.28.1: bare +``CallToolResult`` passes ``structuredContent`` through untouched with no schema +declared, so both paths travel in one shape. Returning ``[Image(...), {...}]`` +instead is the trap that looks right and silently produces **no** +``structuredContent`` at all. + +Previews rather than originals by default because the bytes are base64-encoded +into a single JSON-RPC message: a 12-megapixel original costs an agent its +context window. +""" + +from __future__ import annotations + +import io +from typing import Annotated, Any, Final + +from mcp.server.fastmcp.utilities.types import Image +from mcp.types import CallToolResult, TextContent +from pydantic import Field + +from visionset.kernel.domain import Asset, ImageFormat +from visionset.kernel.ports import THUMBNAIL_FORMAT +from visionset.kernel.services import IngestService, WorkspaceService +from visionset.mcp._resolve import ProjectRef, identifier, resolve_project +from visionset.mcp._workspace import opened_workspace + +SUFFIXES: Final[dict[ImageFormat, str]] = {ImageFormat.JPEG: "jpeg", ImageFormat.PNG: "png"} +"""What ``Image`` wants: the suffix it turns into ``image/``. + +Indexed directly rather than with a fallback, and a parity test asserts it covers +every ``ImageFormat`` member — the ``ProgressCounts`` bargain. A format added to +the enum without an entry fails the suite instead of being served under the wrong +media type or crashing one call in a thousand. +""" + +OCTET_STREAM: Final = "application/octet-stream" +"""What a pre-pipeline asset's bytes are, when nothing recorded a format. + +Inventing a media type would be worse than admitting there is none, which is the +call ``docs/api.md`` already made for the download route. +""" + + +def _payload( + asset: Asset, *, sent: tuple[int | None, int | None], resolution: str +) -> dict[str, Any]: + """The asset's own frame, the frame that was sent, and the factor between them.""" + image_width, image_height = sent + scale = ( + asset.width / image_width + if asset.width is not None and image_width not in (None, 0) + else None + ) + return { + "asset_id": str(asset.id), + "width": asset.width, + "height": asset.height, + "format": None if asset.format is None else asset.format.value, + "content_hash": asset.content_hash, + "image_width": image_width, + "image_height": image_height, + "resolution": resolution, + "scale": scale, + } + + +def _preview(workspace: WorkspaceService, asset: Asset) -> CallToolResult: + with IngestService(workspace).open_thumbnail(asset) as handle: + buffer = io.BytesIO(handle.read()) + measured = workspace.image_processor.probe(buffer) + return CallToolResult( + content=[ + Image(data=buffer.getvalue(), format=SUFFIXES[THUMBNAIL_FORMAT]).to_image_content() + ], + structuredContent=_payload( + asset, sent=(measured.width, measured.height), resolution="thumbnail" + ), + ) + + +def _original(workspace: WorkspaceService, asset: Asset) -> CallToolResult: + with IngestService(workspace).open_content(asset) as handle: + data = handle.read() + payload = _payload(asset, sent=(asset.width, asset.height), resolution="full") + if asset.format is None: + # Nothing recorded a format, so there is no honest media type and `Image` + # cannot label it. Say so in words rather than serving pixels a client + # would have to guess at. + return CallToolResult( + content=[ + TextContent( + type="text", + text=( + f"asset {asset.id} has no recorded image format, so its " + f"{len(data)} bytes are {OCTET_STREAM} and cannot be sent as " + f"image content" + ), + ) + ], + structuredContent=payload, + ) + return CallToolResult( + content=[Image(data=data, format=SUFFIXES[asset.format]).to_image_content()], + structuredContent=payload, + ) + + +def get_asset_image( + project: ProjectRef, + asset_id: Annotated[str, Field(description="The asset to look at, by id.")], + full: Annotated[ + bool, + Field( + description=( + "Return the original bytes instead of the preview. Costly — an " + "original can be many megapixels." + ) + ), + ] = False, +) -> CallToolResult: + """Look at an asset's pixels, so you can annotate what is actually there. + + Returns the image itself plus its measurements. By default it serves the + cached preview, capped at 256 pixels on its long edge, because the bytes + travel base64-encoded in one message and an original would be enormous. Pass + `full=true` when you genuinely need the detail. + + **Write geometry in the asset's own frame.** `width` and `height` are the + asset's true size and are the coordinate system every annotation uses; + `image_width` and `image_height` are what was actually sent. When they differ, + multiply any coordinate you measured on the returned image by `scale` before + passing it to `add_annotations`. Coordinates are never normalized, so an + unscaled preview coordinate is silently wrong rather than obviously wrong. + + Refuses if no preview has been rendered yet, and names `backfill_thumbnails` + as the remedy. An asset with no recorded image format has no honest media + type, so its measurements come back with an explanation instead of pixels. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + asset = IngestService(workspace).asset(resolved.id, identifier(asset_id, what="asset_id")) + return _original(workspace, asset) if full else _preview(workspace, asset) diff --git a/src/visionset/mcp/batches.py b/src/visionset/mcp/batches.py new file mode 100644 index 00000000..d7a27330 --- /dev/null +++ b/src/visionset/mcp/batches.py @@ -0,0 +1,200 @@ +# usage: from visionset.mcp import batches +"""Batch tools: the one-way lifecycle, and the two listings that drive iteration. + +``draft`` → ``approve`` → ``start`` → ``complete``, and there is no way back. +Approval freezes membership, pins the project's active schema version, and cuts +the batch into jobs; nothing after that can return it to a draft, because the +jobs are already partitioned against the pin. + +**There is no ``create_batch`` and no membership editing.** A batch is born from +an ingest. Curating one out of an arbitrary subset of assets has no caller until +a gallery exists to pick that subset in, and after approval the way to exclude an +asset is ``set_asset_progress`` with ``skipped``, not a membership change. +``BatchService`` still has all four methods — this is a decision about the +surface, the same one the REST API and the CLI made. + +``list_batch_jobs`` folds into ``get_batch``: a batch's jobs are how it is worked, +so an agent asking about a batch is about to ask about its jobs. + +``jobs_of`` is the ``BySize`` partition and there is no way to spell +``BySegments``. That variant's own docstring says the caller has already decided +the split, and the only caller holding an exact partition is a program with the +SDK. It is also the one partition that can be *wrong*, with four distinct +refusals, and handing a model the chance to meet all four buys nothing. +""" + +from __future__ import annotations + +from typing import Annotated, Any, Final +from uuid import UUID + +from pydantic import Field + +from visionset import wire +from visionset.kernel.domain import BySize, Partition +from visionset.kernel.services import BatchService, DatasetService, JobService, WorkspaceService +from visionset.mcp._resolve import ProjectRef, identifier, resolve_project +from visionset.mcp._workspace import opened_workspace + +BatchRef = Annotated[str, Field(description="The batch, by id. Batch names are not unique.")] +"""The batch a tool acts on. Module-level for the ``inspect.signature`` reason.""" + +_ACTOR: Final = "mcp" +"""Who the dataset change log records for a promotion made by an agent.""" + + +def _batch_payload(workspace: WorkspaceService, batch_id: UUID) -> dict[str, Any]: + """The batch, its progress and its jobs — the shape three tools return.""" + batches = BatchService(workspace) + batch = batches.get(batch_id) + counts = JobService(workspace).batch_progress(batch.id) + jobs = batches.jobs(batch.id) + return { + **wire.batch(batch, counts), + "jobs": [wire.job(j, batch_id=batch.id) for j in jobs], + } + + +def list_batches(project: ProjectRef) -> dict[str, Any]: + """List a project's batches with where each one's assets have got to. + + The overview of outstanding work: `state` says whether a batch is open, and + `progress.unannotated` says how much is left in it. A batch in + `in_annotation` with unannotated assets is what `next_pending_assets` is for. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + found = BatchService(workspace).list(resolved.id) + jobs = JobService(workspace) + counts = [jobs.batch_progress(b.id) for b in found] + return wire.page( + [wire.batch(b, c) for b, c in zip(found, counts, strict=True)], + ) + + +def get_batch(batch_id: BatchRef) -> dict[str, Any]: + """Read one batch: its state, its schema pin, its progress and its jobs. + + `schema_version` is the contract every annotation in this batch is judged + against, and it is null exactly while the batch is a draft. `jobs` is empty + until the batch is approved — approval is what creates them. + """ + with opened_workspace() as workspace: + return _batch_payload(workspace, identifier(batch_id, what="batch_id")) + + +def approve_batch( + batch_id: BatchRef, + jobs_of: Annotated[ + int | None, + Field( + ge=1, + description=( + "Cut the batch into jobs of this many assets. Omit for one job " + "covering the whole batch." + ), + ), + ] = None, +) -> dict[str, Any]: + """Freeze a batch, pin the project's active schema, and cut it into jobs. + + One-way: there is no route back to `draft`. Approval reads the project's + *current* active schema version and records it on the batch, and a later + `create_schema_version` does not move that pin — so approve after the schema + is the one you want annotations judged against. + + Refuses if the batch has no assets, if it is not a draft, or if the project + has no schema at all. Then call `start_batch` to open it for work. + """ + # `ge=1` on the parameter rather than a check in the body: `BySize.size` is + # `gt=0`, and constructing one with zero raises a pydantic ValidationError, + # which is not a VisionSetError and would never reach the error envelope. + partition: Partition | None = None if jobs_of is None else BySize(size=jobs_of) + with opened_workspace() as workspace: + approved = BatchService(workspace).approve(identifier(batch_id, what="batch_id"), partition) + return _batch_payload(workspace, approved.id) + + +def start_batch(batch_id: BatchRef) -> dict[str, Any]: + """Open an approved batch for annotation. + + Nothing may be written into a batch until this has been called: every + annotation tool and `set_asset_progress` refuse while the batch is not + `in_annotation`. Refuses if the batch has not been approved. + """ + with opened_workspace() as workspace: + started = BatchService(workspace).start(identifier(batch_id, what="batch_id")) + return _batch_payload(workspace, started.id) + + +def complete_batch(batch_id: BatchRef) -> dict[str, Any]: + """Close a batch, once every one of its jobs is complete. + + Derived means recomputed, not automatic: this reads the jobs and refuses + while any is outstanding, so complete the jobs first. A completed batch is + what `promote_batch` requires. + """ + with opened_workspace() as workspace: + completed = BatchService(workspace).complete(identifier(batch_id, what="batch_id")) + return _batch_payload(workspace, completed.id) + + +def list_batch_assets( + batch_id: BatchRef, + limit: Annotated[ + int | None, + Field(ge=1, description="How many assets to return. Omit for all of them."), + ] = None, + offset: Annotated[int, Field(ge=0, description="How many assets to skip.")] = 0, +) -> dict[str, Any]: + """List a batch's assets, with the job each belongs to and its progress. + + The paged view of what is in a batch — a batch of fifty thousand frames is an + ordinary thing, so page it. `total` is the size of the whole batch and does + not change as you page; an offset past the end is an empty page, not an error. + + `job_id` and `progress` are both null exactly while the batch is a draft, + because a draft has no jobs. Use `get_asset_image` on any `id` here to see + the pixels. + """ + with opened_workspace() as workspace: + resolved = identifier(batch_id, what="batch_id") + service = BatchService(workspace) + assets = service.assets(resolved) + # The partition is exact, so each asset appears in at most one job and + # this projection is a lookup rather than a join. Two public reads and no + # new kernel method, which is what the REST listing does too. + placement = { + asset_id: (job.id, progress) + for job in service.jobs(resolved) + for asset_id, progress in job.progress.items() + } + # `limit` bounds the *response*, not the read. The kernel has no windowed + # read, so this slices a full list and `total` stays the size of the whole + # batch — page until you have seen `total` items, not until it moves. + window = assets[offset:] if limit is None else assets[offset : offset + limit] + items = [ + wire.batch_asset(a, job_id=job_id, progress=progress) + for a in window + for job_id, progress in [placement.get(a.id, (None, None))] + ] + return {"items": items, "total": len(assets)} + + +def promote_batch(batch_id: BatchRef) -> dict[str, Any]: + """Move a completed batch's finished assets into the project's dataset. + + The dataset is the trunk every release is cut from, so this is the step + between annotating and publishing. Only `annotated` and `accepted` assets + travel — a `skipped` one was a decision and it is honoured. Annotations come + along with their assets. + + A union against what is already there: promoting the same batch twice adds + nothing the second time and records nothing. Refuses if the batch is not + complete. + """ + with opened_workspace() as workspace: + promoted = DatasetService(workspace).promote( + identifier(batch_id, what="batch_id"), actor=_ACTOR + ) + return wire.page([wire.asset(a) for a in promoted]) diff --git a/src/visionset/mcp/datasets.py b/src/visionset/mcp/datasets.py new file mode 100644 index 00000000..b3da3794 --- /dev/null +++ b/src/visionset/mcp/datasets.py @@ -0,0 +1,45 @@ +# usage: from visionset.mcp import datasets +"""Dataset tools: what is in the trunk. + +One tool. The dataset is 1:1 with its project and is reached through it here, +rather than by a ``dataset_id`` an agent would have to fetch and carry — which is +also why ``get_dataset`` and ``get_project_dataset`` fold into ``get_project``. + +Three parity candidates are **dropped** rather than folded, because no agent +calls them. ``list_dataset_assets`` walks the trunk, and the annotation loop +iterates *batches*; ``list_dataset_changes`` is an audit record a person reads +when they want to know who removed something; ``remove_dataset_asset`` is +curation, which is a judgement about what a dataset should contain rather than a +step in producing one. ``promote_batch``, the write that fills the trunk, lives +in ``batches`` because ``DatasetService.promote`` takes a *batch* id. +""" + +from __future__ import annotations + +from typing import Any + +from visionset import wire +from visionset.kernel.services import DatasetService, ProjectService +from visionset.mcp._resolve import ProjectRef, resolve_project +from visionset.mcp._workspace import opened_workspace + + +def dataset_stats(project: ProjectRef) -> dict[str, Any]: + """Count what is in a project's dataset, class by class. + + The "is this dataset ready to train on" question. `classes` gives both totals + per class and they answer different things: a thousand labels over a thousand + images and the same thousand over ten are the same `annotations` and a very + different dataset. A class the schema declares but nobody has used does not + appear at all. + + Derived on every call from current membership, so it moves as batches are + promoted. A release freezes its own counts at publication and those never + move. `asset_count` minus `annotated_asset_count` is how many promoted assets + carry no labels. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + dataset = ProjectService(workspace).get_dataset(resolved.id) + stats = DatasetService(workspace).stats(dataset.id) + return wire.dataset_stats(stats) diff --git a/src/visionset/mcp/formats.py b/src/visionset/mcp/formats.py new file mode 100644 index 00000000..d9cc1565 --- /dev/null +++ b/src/visionset/mcp/formats.py @@ -0,0 +1,30 @@ +# usage: from visionset.mcp import formats +"""``list_formats`` — which exporters are installed, and which of them lose things. + +Discovery is over the ``visionset.formats`` entry-point group, so a third-party +distribution's exporter is indistinguishable from a built-in here. Nothing is +cached: the alternative is a server that has to be restarted after an install. +""" + +from __future__ import annotations + +from typing import Any + +from visionset import wire +from visionset.formats.registry import exporters + + +def list_formats() -> dict[str, Any]: + """List the export formats installed in this VisionSet, and whether each is lossy. + + Call this before `export_release` — the `name` here is exactly the string + that tool's `format` parameter takes, and a name that is not in this list is + refused. + + `lossy` is declared by the format itself, not measured against your data: it + says the format cannot express everything VisionSet can hold, whether or not + today's release happens to use the part that would be lost. Exporting in one + requires `allow_lossy=true`. + """ + installed = exporters() + return wire.page([wire.export_format(installed[name]) for name in sorted(installed)]) diff --git a/src/visionset/mcp/jobs.py b/src/visionset/mcp/jobs.py new file mode 100644 index 00000000..df7b6bd6 --- /dev/null +++ b/src/visionset/mcp/jobs.py @@ -0,0 +1,137 @@ +# usage: from visionset.mcp import jobs +"""Job tools: the annotation loop an agent actually drives. + +A job is one segment of an approved batch. ``start_job`` → +``next_pending_assets`` → look at the pixels → ``add_annotations`` → +``set_asset_progress`` where nothing is there to label → ``complete_job``. + +``get_job_progress`` folds into ``get_job``: the counts *are* what a caller wants +a job for, and a second tool to fetch them is a round trip for a field. + +``next_pending_assets`` is the iteration primitive and the reason this loop +terminates. It returns only ``unannotated`` assets in stored order, so calling it +after each write walks the job exactly once with no bookkeeping on the agent's +side. +""" + +from __future__ import annotations + +from typing import Annotated, Any + +from pydantic import Field + +from visionset import wire +from visionset.kernel.domain import AssetProgress +from visionset.kernel.services import JobService +from visionset.mcp._resolve import identifier +from visionset.mcp._workspace import opened_workspace + +JobRef = Annotated[str, Field(description="The annotation job, by id.")] +"""The job a tool acts on. Module-level for the ``inspect.signature`` reason.""" + + +def _job_payload(service: JobService, job_id: Any) -> dict[str, Any]: + """The job, the batch it belongs to, and its counts — the shape three tools return.""" + job = service.get(job_id) + batch = service.batch(job.id) + return { + **wire.job(job, batch_id=batch.id), + "batch_state": batch.state.value, + "schema_version": batch.schema_version, + "progress": wire.progress_counts(service.job_progress(job.id)), + } + + +def get_job(job_id: JobRef) -> dict[str, Any]: + """Read a job: its state, its counts, and the batch and schema it answers to. + + `schema_version` is the contract annotations written here are judged against; + it comes from the batch's pin, not from the project's current schema. + `batch_state` matters because nothing may be written unless it is + `in_annotation` — if it is `approved`, call `start_batch` first. + + `progress.unannotated` is how much is left; when it and `review_pending` are + both zero the job can be completed. + """ + with opened_workspace() as workspace: + return _job_payload(JobService(workspace), identifier(job_id, what="job_id")) + + +def start_job(job_id: JobRef) -> dict[str, Any]: + """Mark a job as being worked on. + + Refuses unless the job's batch is already `in_annotation`. Starting a job + does not start its batch — that is `start_batch`, and it comes first. + """ + with opened_workspace() as workspace: + service = JobService(workspace) + started = service.start(identifier(job_id, what="job_id")) + return _job_payload(service, started.id) + + +def complete_job(job_id: JobRef) -> dict[str, Any]: + """Close a job, once every one of its assets has been settled. + + Settled means `annotated`, `skipped` or `accepted`. An asset still + `unannotated` or `review_pending` blocks this, and the remedy is either to + annotate it or to `set_asset_progress` it to `skipped`. + + Completing a job does not complete its batch: `complete_batch` derives that + from all the jobs, and is a separate call. + """ + with opened_workspace() as workspace: + service = JobService(workspace) + completed = service.complete(identifier(job_id, what="job_id")) + return _job_payload(service, completed.id) + + +def next_pending_assets( + job_id: JobRef, + count: Annotated[ + int, Field(ge=1, description="How many assets to return. Must be at least 1.") + ] = 10, +) -> dict[str, Any]: + """Get the next assets in a job that nobody has annotated yet. + + The loop primitive: call it, annotate what comes back, call it again. It + returns only `unannotated` assets, in the job's own stored order, so an asset + stops appearing once it has been annotated or skipped and the walk terminates + without you tracking position. + + An empty `items` means every asset in the job has been settled and the job + can be completed. Each `id` is what `get_asset_image` and `add_annotations` + take. + """ + with opened_workspace() as workspace: + assets = JobService(workspace).next_pending(identifier(job_id, what="job_id"), count) + return wire.page([wire.asset(a) for a in assets]) + + +def set_asset_progress( + job_id: JobRef, + asset_id: Annotated[str, Field(description="The asset within that job, by id.")], + progress: Annotated[ + AssetProgress, + Field( + description=( + "The state to move the asset to. Use 'skipped' for an asset with nothing to label." + ) + ), + ], +) -> dict[str, Any]: + """Record where one asset of a job has got to, without writing annotations. + + Chiefly how you say "there is nothing in this image to label": mark it + `skipped` and it stops blocking `complete_job` and never enters the dataset. + Writing annotations moves an asset to `annotated` on its own, so you do not + need this for the ordinary case. + + Not every move is legal — `accepted` is terminal, and `review_pending` can + only be reached from `annotated`. Marking an asset with the state it already + holds does nothing and is not an error. Refuses if the job's batch is not + `in_annotation`. + """ + with opened_workspace() as workspace: + resolved_asset = identifier(asset_id, what="asset_id") + JobService(workspace).mark(identifier(job_id, what="job_id"), resolved_asset, progress) + return wire.asset_progress(resolved_asset, progress) diff --git a/src/visionset/mcp/main.py b/src/visionset/mcp/main.py index 8e3344f2..a626a998 100644 --- a/src/visionset/mcp/main.py +++ b/src/visionset/mcp/main.py @@ -1,135 +1,133 @@ -"""MCP server over stdio. Run with: python -m visionset.mcp.main - -Every tool is a stub returning a structured `not_implemented` error until the -kernel SDK lands. Convention (starts now): tools that mutate or write anything -declare `confirm: bool = False` and refuse to act while it is false. +"""MCP server over stdio. Run with: ``visionset mcp``, or ``python -m visionset.mcp.main``. + +The fourth client of the kernel SDK, beside the REST API, the CLI and the SDK +itself. Every tool is a thin mapping onto one or two service calls; nothing is +decided here that the kernel has not already decided. + +**Thirty-three tools, out of fifty candidates.** Each REST task from #27 to #30 +recorded which MCP tools its capability implied, and this is the sweep that +settled them one by one. 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 neighbouring tool already covers. What +folded, what was dropped and why is argued in ``docs/mcp.md`` and in each +module's own docstring. + +**Registration is this table, not a decorator at each definition site.** The CLI's +rule, for the CLI's reason: ``@server.tool()`` inside ``projects.py`` would make +that module import this one, which imports it. Doing it here also puts every +shipped tool on one screen, and gives the three cross-cutting decisions exactly +one place to live — + +* ``guarded`` wraps every body, so a kernel refusal arrives as the error envelope + rather than as an exception whose text FastMCP would ship to the client anyway, + prefixed and unstructured; +* ``inspect.cleandoc`` is passed as ``description=`` because FastMCP otherwise + ships ``__doc__`` **raw** — indentation and all — into the listing an agent + reads; +* ``ToolAnnotations`` says whether a tool reads or writes. They are *hints* and + enforce nothing; ``confirm`` is what enforces. So + ``tests/mcp/test_registration.py`` asserts the two agree rather than trusting + that they do. + +A duplicate name would not raise: FastMCP logs a warning and silently discards +the second registration. That same test asserts the server lists exactly as many +tools as this table holds. + +**No ``from __future__ import annotations`` here**, and it is not an oversight. +That import binds the name ``annotations`` to a ``__future__._Feature``, which +``from visionset.mcp import annotations`` below would then shadow — reported by +mypy as an incompatible import, exactly as it was for ``server/routes``. Nothing +in this module needs deferred evaluation. """ -from __future__ import annotations - -from typing import Any +import inspect +from collections.abc import Callable +from typing import Any, Final from mcp.server.fastmcp import FastMCP +from mcp.types import ToolAnnotations + +from visionset.mcp import ( + annotations, + assets, + batches, + datasets, + formats, + jobs, + projects, + releases, + schemas, + sources, +) +from visionset.mcp._errors import guarded server = FastMCP("visionset") - -def _not_implemented(tool: str) -> dict[str, Any]: - return { - "error": { - "code": "not_implemented", - "message": f"'{tool}' is not implemented yet; the kernel SDK lands in a later session.", - } - } - - -def _confirmation_required(tool: str) -> dict[str, Any]: - return { - "error": { - "code": "confirmation_required", - "message": f"'{tool}' mutates state; call again with confirm=true to proceed.", - } - } - - -# --- read-only tools --------------------------------------------------------- - - -@server.tool() -def list_projects() -> dict[str, Any]: - """List all projects in the workspace.""" - return _not_implemented("list_projects") - - -@server.tool() -def get_project_status(project_id: str) -> dict[str, Any]: - """Get the status summary of a project.""" - return _not_implemented("get_project_status") - - -@server.tool() -def get_schema(project_id: str) -> dict[str, Any]: - """Get the current annotation schema of a project.""" - return _not_implemented("get_schema") - - -@server.tool() -def get_ingest_progress(ingest_job_id: str) -> dict[str, Any]: - """Get the progress of an ingest job.""" - return _not_implemented("get_ingest_progress") - - -@server.tool() -def list_jobs(project_id: str) -> dict[str, Any]: - """List annotation jobs in a project.""" - return _not_implemented("list_jobs") - - -@server.tool() -def get_annotation_progress(job_id: str) -> dict[str, Any]: - """Get per-asset annotation progress for a job.""" - return _not_implemented("get_annotation_progress") - - -@server.tool() -def dataset_stats(dataset_id: str) -> dict[str, Any]: - """Get statistics (class balance, counts) for a dataset.""" - return _not_implemented("dataset_stats") - - -# --- mutating tools: the confirm-parameter convention starts here ------------ - - -@server.tool() -def create_project(name: str, confirm: bool = False) -> dict[str, Any]: - """Create a new project. Requires confirm=true.""" - if not confirm: - return _confirmation_required("create_project") - return _not_implemented("create_project") - - -@server.tool() -def ingest_source(project_id: str, uri: str, confirm: bool = False) -> dict[str, Any]: - """Ingest a source (e.g. a local folder) into a project. Requires confirm=true.""" - if not confirm: - return _confirmation_required("ingest_source") - return _not_implemented("ingest_source") - - -@server.tool() -def partition_batch(batch_id: str, task_count: int, confirm: bool = False) -> dict[str, Any]: - """Partition a batch into task groups. Requires confirm=true.""" - if not confirm: - return _confirmation_required("partition_batch") - return _not_implemented("partition_batch") - - -@server.tool() -def add_annotations( - job_id: str, annotations: list[dict[str, Any]], confirm: bool = False -) -> dict[str, Any]: - """Add annotations to a job (provenance will be 'model' or 'import'). Requires confirm=true.""" - if not confirm: - return _confirmation_required("add_annotations") - return _not_implemented("add_annotations") +READS: Final = ToolAnnotations(readOnlyHint=True) +"""Reads rows and changes nothing.""" + +WRITES: Final = ToolAnnotations(readOnlyHint=False, destructiveHint=False) +"""Changes state, but only ever adds to it or advances it.""" + +DESTROYS: Final = ToolAnnotations(readOnlyHint=False, destructiveHint=True) +"""Removes something that cannot be recovered. Carries ``confirm``.""" + +TOOLS: Final[tuple[tuple[Callable[..., Any], ToolAnnotations], ...]] = ( + # Registration is in cycle order — make a project, give it a schema, put + # images in it, work through them, promote, publish, export — because that is + # the order an agent meets them in, and a listing that reads as the workflow + # is one a model can plan against. + (projects.create_project, WRITES), + (projects.list_projects, READS), + (projects.get_project, READS), + (projects.delete_project, DESTROYS), + (schemas.get_schema, READS), + (schemas.preview_schema_change, READS), + (schemas.create_schema_version, WRITES), + (sources.ingest, WRITES), + (sources.list_sources, READS), + (sources.backfill_thumbnails, WRITES), + (batches.list_batches, READS), + (batches.get_batch, READS), + (batches.approve_batch, WRITES), + (batches.start_batch, WRITES), + (batches.list_batch_assets, READS), + (jobs.get_job, READS), + (jobs.start_job, WRITES), + (jobs.next_pending_assets, READS), + (assets.get_asset_image, READS), + (annotations.list_asset_annotations, READS), + (annotations.add_annotations, WRITES), + (annotations.update_annotations, WRITES), + (annotations.delete_annotations, WRITES), + (jobs.set_asset_progress, WRITES), + (jobs.complete_job, WRITES), + (batches.complete_batch, WRITES), + (batches.promote_batch, WRITES), + (datasets.dataset_stats, READS), + (releases.publish_release, WRITES), + (releases.list_releases, READS), + (releases.verify_release, READS), + (formats.list_formats, READS), + (releases.export_release, WRITES), +) +"""Every shipped tool, with what it does to the workspace. + +``delete_annotations`` is ``WRITES`` rather than ``DESTROYS``, on purpose: it +takes no ``confirm``, because removing a label is the annotator edit loop and the +guard is the batch gate. In this surface ``destructiveHint`` and ``confirm`` mean +the same thing, and the registration test holds them to it. +""" -@server.tool() -def publish_release(dataset_id: str, tag: str, confirm: bool = False) -> dict[str, Any]: - """Publish an immutable release of a dataset. Requires confirm=true.""" - if not confirm: - return _confirmation_required("publish_release") - return _not_implemented("publish_release") +def _register() -> None: + for tool, hints in TOOLS: + server.tool(description=inspect.cleandoc(tool.__doc__ or ""), annotations=hints)( + guarded(tool) + ) -@server.tool() -def export_release( - release_id: str, format_name: str, dest: str, confirm: bool = False -) -> dict[str, Any]: - """Export a release to disk in the given format. Requires confirm=true.""" - if not confirm: - return _confirmation_required("export_release") - return _not_implemented("export_release") +_register() def main() -> None: diff --git a/src/visionset/mcp/projects.py b/src/visionset/mcp/projects.py new file mode 100644 index 00000000..ca774fd7 --- /dev/null +++ b/src/visionset/mcp/projects.py @@ -0,0 +1,111 @@ +# usage: from visionset.mcp import projects +"""Project tools: create, list, read, delete. + +``get_project`` folds three parity candidates into one call. ``get_project``, +``get_project_dataset`` and ``get_dataset`` are three round trips for facts an +agent invariably wants together — the dataset id is the handle every release tool +needs, and it is 1:1 with the project, so making an agent fetch it separately buys +nothing but a chance to lose it. The progress counts come along for the same +reason: "what is in this project and how far along is it" is one question. + +``rename_project`` is **not** here. It is the only project write that changes +nothing an agent can observe going wrong, and a tool that exists only so a model +can fix a typo in a name it chose is list-padding. + +``delete_project`` **is** here, and it is the only tool in the whole surface that +destroys data. It carries ``confirm``. +""" + +from __future__ import annotations + +from typing import Annotated, Any + +from pydantic import Field + +from visionset import wire +from visionset.kernel.services import JobService, ProjectService +from visionset.mcp._resolve import ProjectRef, resolve_project +from visionset.mcp._workspace import opened_workspace + + +def create_project( + name: Annotated[ + str, + Field(description="A name for the project, unique in this workspace case-insensitively."), + ], + description: Annotated[ + str | None, Field(description="Optional free text describing the project.") + ] = None, +) -> dict[str, Any]: + """Create a project, together with the empty dataset that is its trunk. + + Use this first: every other tool is scoped to a project. Returns the project + and its dataset id. Refuses with a message if the name is blank or already + taken — names are compared case-insensitively, so "Road-Signs" collides with + "road-signs". A new project has no annotation schema; call + `create_schema_version` before ingesting anything you intend to label. + """ + with opened_workspace() as workspace: + projects = ProjectService(workspace) + created = projects.create(name, description) + dataset = projects.get_dataset(created.id) + return {"project": wire.project(created), "dataset": wire.dataset(dataset)} + + +def list_projects() -> dict[str, Any]: + """List every project in this workspace, in creation order. + + Use this to discover what exists before naming one. Returns + `{"items": [...], "total": n}`; an empty workspace is `total: 0` and not an + error. + """ + with opened_workspace() as workspace: + projects = ProjectService(workspace).list() + return wire.page([wire.project(p) for p in projects]) + + +def get_project(project: ProjectRef) -> dict[str, Any]: + """Read a project, its dataset id, and how far its annotation work has got. + + The one call that answers "what is this and where does it stand". `progress` + counts every asset in the project by state, across all its batches, so + `unannotated` is what is left to do. `dataset.id` is the handle + `dataset_stats`, `publish_release` and `list_releases` take. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + dataset = ProjectService(workspace).get_dataset(resolved.id) + counts = JobService(workspace).project_progress(resolved.id) + return { + "project": wire.project(resolved), + "dataset": wire.dataset(dataset), + "progress": wire.progress_counts(counts), + } + + +def delete_project( + project: ProjectRef, + confirm: Annotated[ + bool, + Field( + description=( + "Must be true to actually delete. False returns a refusal and changes nothing." + ) + ), + ] = False, +) -> dict[str, Any]: + """Delete a project and everything under it. Destructive; requires `confirm=true`. + + Takes with it the schema versions, sources, assets, annotations, batches, + jobs, the dataset and every release — irreversibly, and with no undo. The + image bytes themselves survive in the workspace's blob store, because content + is shared and addressed by hash, but nothing points at them any more. + + Called without `confirm=true` it changes nothing and tells you so; that + refusal is the intended way to check what you are about to do. An unknown + project is reported as missing whether or not `confirm` was passed. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + ProjectService(workspace).delete(resolved.id, confirm=confirm) + return {"deleted": wire.project(resolved)} diff --git a/src/visionset/mcp/releases.py b/src/visionset/mcp/releases.py new file mode 100644 index 00000000..2d0a9e1e --- /dev/null +++ b/src/visionset/mcp/releases.py @@ -0,0 +1,173 @@ +# usage: from visionset.mcp import releases +"""Release tools: freeze a dataset, check it, and write it out in a format. + +A release is the only truly immutable artifact VisionSet makes. Its manifest is a +pure function of content — no timestamp, no tag, no id inside the document — so +publishing an unchanged dataset twice produces byte-identical manifests that +share one blob. + +**Releases are addressed by project and tag**, never by a release id an agent +would have to carry. A tag is unique per dataset and it is what a person picked; +it is also the one comparison in the system that is **case-sensitive**, opposite +to a project name, and that rule is a kernel read rather than something spelled +here. + +``get_release`` folds into ``list_releases``: the listing already carries every +published field, so a second tool to fetch one row is a round trip for nothing. + +Two candidates are **dropped**. ``get_release_manifest`` returns the whole frozen +document — every asset, every annotation — which for a real dataset is a token +bill an agent cannot afford and does not need, since ``verify_release`` answers +"is it intact" and ``export_release`` writes the contents where a trainer can +read them. ``get_release_assignment`` returns the train/val/test folds as three +lists of ids, which is the same information ``export_release`` puts on disk in +the form anything downstream actually consumes. + +**Export is synchronous**, a stated limit rather than an oversight: launch-and- +poll needs a row to poll, a row needs a table, and a table needs a migration. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Annotated, Any + +from pydantic import Field + +from visionset import wire +from visionset.formats.registry import exporter +from visionset.kernel.domain import SplitRecipe +from visionset.kernel.services import ProjectService, ReleaseService +from visionset.mcp._errors import refused +from visionset.mcp._resolve import ProjectRef, resolve_project, resolve_release +from visionset.mcp._workspace import opened_workspace + +TagRef = Annotated[str, Field(description="The release tag. Compared case-sensitively.")] +"""Module-level for the ``inspect.signature`` reason.""" + + +def publish_release( + project: ProjectRef, + tag: TagRef, + split: Annotated[ + SplitRecipe | None, + Field( + description=( + "How to cut the release for training. Fractions must sum to 1.0. " + "Omit to publish without one." + ) + ), + ] = None, +) -> dict[str, Any]: + """Freeze the project's dataset as an immutable, tagged release. + + Snapshots whatever is in the dataset right now — every promoted asset and a + *copy* of its annotations, so later edits do not reach back into a published + release. The manifest is hashed, and that hash is what `verify_release` + checks against later. + + `split` is stored, not applied: nothing is moved or copied per fold at + publication. The assignment is computed deterministically from asset content + hashes, so the same recipe over the same content always gives the same folds + and `export_release` writes them out. + + Refuses if the dataset is empty, if the tag is blank, or if the tag is + already used in this dataset — tags are compared case-sensitively, so `v1.0` + and `V1.0` are two different releases. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + dataset = ProjectService(workspace).get_dataset(resolved.id) + published = ReleaseService(workspace).publish(dataset.id, tag, split=split) + return wire.release(published) + + +def list_releases(project: ProjectRef) -> dict[str, Any]: + """List a project's releases, newest last, with everything each one publishes. + + Each row carries its `manifest_hash`, the schema version it froze, its asset + and annotation counts and its split recipe — so this answers "what has been + published and what is in it" without a second call per release. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + dataset = ProjectService(workspace).get_dataset(resolved.id) + found = ReleaseService(workspace).list(dataset.id) + return wire.page([wire.release(r) for r in found]) + + +def verify_release(project: ProjectRef, tag: TagRef) -> dict[str, Any]: + """Re-read and re-hash everything a release names, and report what is wrong. + + The integrity check: it re-hashes the manifest and then every asset blob the + manifest lists, so it detects bit rot and missing files that a listing cannot. + Expect it to take a while on a large release. + + `ok` true means the manifest is intact, every blob is present and hashes + correctly, and the cached counts on the release row agree with the document. + `missing` is blobs that are gone, `corrupt` is blobs present whose bytes no + longer hash to their name, and `cache_mismatches` names a row field that + disagrees with the manifest — which is a defect in the build that wrote it, + not damage on disk. + + If the manifest itself fails its own hash, nothing else can be trusted, so + `manifest_intact` is false and `checked` is 0. + """ + with opened_workspace() as workspace: + release = resolve_release(workspace, project, tag) + report = ReleaseService(workspace).verify(release.id) + return wire.release_verification(report) + + +def export_release( + project: ProjectRef, + tag: TagRef, + format: Annotated[str, Field(description="An installed exporter's name. See `list_formats`.")], + dest: Annotated[ + str, + Field(description="An absolute directory path on this machine to write into."), + ], + allow_lossy: Annotated[ + bool, + Field( + description=( + "Must be true to export in a format that cannot carry everything " + "this release holds." + ) + ), + ] = False, +) -> dict[str, Any]: + """Write a release to a local directory in one of the installed formats. + + Blocks until the export finishes and returns a description of what landed — + the bytes stay on disk, which is the point: whatever trains on this reads the + directory, not this call's answer. + + `dest` is created if it does not exist and is **not emptied first**, so + `file_count` and `total_bytes` describe the directory afterwards, which + equals what this run wrote only when the directory was fresh. Point separate + exports at separate directories. + + A format that cannot express everything the release holds — one that carries + boxes but not polygons, say — is declared lossy by the format itself, and + refuses until you pass `allow_lossy=true`. That is a different gate from + `confirm`: nothing is destroyed, the release stays exactly as it was, and + what you are consenting to is an incomplete copy. + """ + destination = Path(dest) + # An exporter writes into `dest` and creates it if missing, so a path that + # exists as a *file* is the one shape that cannot work and would surface as a + # bare OSError from inside a plugin rather than as a refusal. + if destination.exists() and not destination.is_dir(): + return refused(f"dest must be a directory, and {dest} is a file") + + with opened_workspace() as workspace: + release = resolve_release(workspace, project, tag) + # `pick`, through `exporter()`, rather than indexing the registry: a + # `KeyError` is outside the VisionSetError tree, so a mistyped format name + # has to arrive as a refusal that names the installed ones. + plugin = exporter(format) + result = ReleaseService(workspace).export( + release.id, plugin, destination, allow_lossy=allow_lossy + ) + return wire.export_result(result) diff --git a/src/visionset/mcp/schemas.py b/src/visionset/mcp/schemas.py new file mode 100644 index 00000000..ebc37424 --- /dev/null +++ b/src/visionset/mcp/schemas.py @@ -0,0 +1,152 @@ +# usage: from visionset.mcp import schemas +"""Schema tools: read the contract, propose a change, apply one. + +**The domain's own ``LabelClass`` is the parameter type**, not a hand-written +body. That is the opposite call from the REST surface, and the reason is that the +two surfaces publish their input schemas to different readers. FastAPI copies a +model's docstring verbatim into ``openapi.json`` and turns a PEP 695 alias into a +named component, so ``server/models.py`` keeps its own spellings; FastMCP puts +the same docstrings into ``$defs`` on the tool's ``inputSchema``, where they are +the best guidance an agent gets about what a class *is*. Re-spelling the model +here would throw that away and add a second definition to keep in step. + +**The one wart it inherits, stated rather than hidden**: a discriminated union's +tag carries a default in the domain — ``LabelClass.geometry`` does not, but +``Geometry`` and ``Partition`` do — so the generated schema shows ``type`` as +optional while pydantic needs it in the input dict to pick a variant. #29 fixed +that on the wire by dropping the defaults from its own bodies; here it is +answered in the tool description and pinned by a test, because the alternative is +the re-spelling this module exists to avoid. + +``list_schema_versions`` folds into ``get_schema``: which versions exist is one +list of integers, and shipping a tool to fetch it is a round trip for something +that fits in the answer to "what is the schema". + +``preview_schema_change`` gives ``SchemaService.preview`` its **first caller**. +It has been in the kernel since #6 and unrouted since #27 for want of one. An +agent is exactly the caller it was waiting for: plan-before-apply is how a model +finds out that a change it is about to make would orphan somebody's work, and +finding out *before* is the whole point. +""" + +from __future__ import annotations + +from typing import Annotated, Any + +from pydantic import Field + +from visionset import wire +from visionset.kernel.domain import LabelClass +from visionset.kernel.services import SchemaService +from visionset.mcp._resolve import ProjectRef, resolve_project +from visionset.mcp._workspace import opened_workspace + +ClassesParam = Annotated[ + list[LabelClass], + Field( + description=( + "The complete list of label classes the new version declares. This is " + "the whole contract, not a patch: a class left out is a class removed." + ) + ), +] +"""The proposed classes, for the two tools that take a whole schema. + +Module-level for the ``inspect.signature`` reason :data:`ProjectRef` is. +""" + + +def get_schema( + project: ProjectRef, + version: Annotated[ + int | None, + Field(ge=1, description="A specific version to read. Omit for the active (highest) one."), + ] = None, +) -> dict[str, Any]: + """Read a project's annotation schema — which classes exist and what each may carry. + + Read this before writing any annotation: `add_annotations` judges every label + against the version its batch pinned, and a class name or geometry the schema + does not declare is refused. `available_versions` lists every version ever + created, oldest first; `active_version` is the highest, which is what the next + `approve_batch` will pin. + + A project that has never had `create_schema_version` called on it has no + schema at all, and this reports that rather than inventing an empty one. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + service = SchemaService(workspace) + versions = service.list_versions(resolved.id) + found = ( + service.get_active(resolved.id) + if version is None + else service.get(resolved.id, version) + ) + return { + "schema": wire.schema_version(found), + "active_version": versions[-1].version, + "available_versions": [v.version for v in versions], + } + + +def preview_schema_change(project: ProjectRef, classes: ClassesParam) -> dict[str, Any]: + """Say what applying these classes would change, without applying anything. + + Writes nothing. Use it before `create_schema_version` whenever you are + changing an existing schema rather than creating the first one — it is the + only way to find out that a change is destructive without attempting it. + + `is_destructive` true means the proposal narrows the contract: a class or an + attribute is gone, or a geometry moved. `destructive_classes` names them. + Applying it then needs `allow_destructive=true` — unless annotations already + exist under one of those classes, in which case `create_schema_version` + refuses outright and no flag overrides it. Adding classes or optional + attributes is additive and needs no flag. + + Each entry of `classes` must carry `geometry` as one of the declared geometry + types; matching against the current version is by exact class name, so + renaming a class reads here as one removal plus one addition. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + diff = SchemaService(workspace).preview(resolved.id, classes) + return wire.schema_diff(diff) + + +def create_schema_version( + project: ProjectRef, + classes: ClassesParam, + allow_destructive: Annotated[ + bool, + Field( + description=( + "Must be true to apply a change that removes or narrows something. " + "Has no effect on an additive change." + ) + ), + ] = False, +) -> dict[str, Any]: + """Create the next schema version from a complete list of classes. + + Versions are 1..N and never edited or deleted; this always inserts a new one + and the highest becomes active. Batches already approved keep the version + they pinned, so this does not retroactively change how existing work is + judged. + + Send the whole contract every time — a class omitted is a class removed. + Call `preview_schema_change` first if you are not creating the first version. + + Three refusals to expect. A class bound to a geometry VisionSet has not + implemented is rejected outright. A narrowing change is rejected until you + pass `allow_destructive=true`. And a narrowing change that would orphan + annotations already written under an affected class is rejected with **no** + override at all — the remedy there is to keep the class, not to force the + change. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + created = SchemaService(workspace).create_version( + resolved.id, classes, allow_destructive=allow_destructive + ) + return wire.schema_version(created) diff --git a/src/visionset/mcp/sources.py b/src/visionset/mcp/sources.py new file mode 100644 index 00000000..211f332b --- /dev/null +++ b/src/visionset/mcp/sources.py @@ -0,0 +1,155 @@ +# usage: from visionset.mcp import sources +"""Ingest tools: one path in, one batch out. And what has been registered before. + +**Four parity candidates collapse into ``ingest``.** ``register_image_source``, +``register_video_source`` and ``start_ingest`` are three tools describing one +intention, and the split exists in the kernel for a reason that does not reach +this far up: ``SourceService`` has two registration methods because a clip needs +a rate and a probe while a folder needs neither, and ``IngestService`` has one +``ingest`` because by then the source already carries the kind, the path and the +rate. An agent holding a path should not have to say which of the two it has — +the dispatch is ``path.is_dir()``, exactly as ``visionset ingest`` does it. + +**A local path, never an upload.** ``server/uploads.py`` exists because HTTP has +bytes where the kernel has paths; an agent runs beside the workspace and has the +filesystem, so there is nothing to stage and that module must not grow a caller +here. + +**The run is synchronous and there is nothing to poll**, which is why +``get_ingest_job``, ``list_ingest_jobs`` and ``resume_ingest`` are not tools. A +stdio server has no background worker: something has to do the decode, and +"resume" done by the agent would block for exactly as long as doing it in the +first place. The finished job comes back in the answer. If a call is cut off part +way, the remedy is to 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 gave the CLI no +``--resume``. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Annotated, Any + +from pydantic import Field + +from visionset import wire +from visionset.kernel.ports import DEFAULT_EXTRACTION_FPS +from visionset.kernel.services import IngestService, SourceService +from visionset.mcp._errors import refused +from visionset.mcp._resolve import ProjectRef, resolve_project +from visionset.mcp._workspace import opened_workspace + + +def ingest( + project: ProjectRef, + path: Annotated[ + str, + Field( + description=( + "An absolute path on this machine: a directory of still images, or " + "a single video file." + ) + ), + ], + fps: Annotated[ + float | None, + Field( + description=( + "Frames per second to extract. Video sources only; defaults to " + f"{DEFAULT_EXTRACTION_FPS}. Must be greater than zero." + ) + ), + ] = None, + batch_name: Annotated[ + str | None, + Field(description="Name the batch this run fills. Defaults to the source's own name."), + ] = None, +) -> dict[str, Any]: + """Register a source and read it into one batch. Blocks until the run finishes. + + A directory is read top level only, in filename order, with no filter on the + suffix — anything that is not a usable image is reported in `failures` and + the run carries on. A video file is decomposed into frames at `fps`, and the + rate is part of what the source *is*: the same clip registered at 1 and at 5 + is two sources, deliberately. + + Assets are addressed by content, so ingesting the same bytes twice yields one + asset. `created` counts new assets and `deduplicated` counts ones already + known; both went into the batch. That is also why re-running this after an + interrupted call is safe and nearly free. + + The `batch_id` it returns is what `approve_batch` takes next. A long video + can make this call take minutes; there is no progress to poll from here. + + Refuses before doing any work if the path does not exist, if `fps` is not + positive, or if `fps` was given for a directory of stills. + """ + source_path = Path(path) + # Three refusals the kernel raises *outside* the VisionSetError tree, so + # `guarded` would not catch them and the client would get a traceback's text + # instead of an envelope. `canonical_path` resolves strictly + # (FileNotFoundError), `register_images` wants a directory + # (NotADirectoryError), and `register_video` refuses a non-positive rate with + # a bare ValueError. + if not source_path.exists(): + return refused(f"no such path: {path}") + if fps is not None and fps <= 0: + return refused("fps must be greater than zero") + if fps is not None and source_path.is_dir(): + return refused(f"fps applies to a video source, and {path} is a directory of stills") + + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + service = SourceService(workspace) + if source_path.is_dir(): + registered = service.register_images(resolved.id, source_path) + else: + registered = service.register_video( + resolved.id, + source_path, + extraction_fps=DEFAULT_EXTRACTION_FPS if fps is None else fps, + ) + result = IngestService(workspace).ingest(registered.id, batch_name=batch_name) + return { + "source": wire.source(registered), + "job_id": str(result.job_id), + "batch_id": str(result.batch_id), + "created": result.created, + "deduplicated": result.deduplicated, + "failed": result.failed, + "failures": [wire.ingest_failure(f) for f in result.failures], + } + + +def list_sources(project: ProjectRef) -> dict[str, Any]: + """List the origins registered in a project — the folders and clips it was built from. + + Use it to see what has already been ingested before ingesting again. `name` + is the path's last component only; the full path is not published, because it + describes this machine's disk and not anything a caller can act on. A video + source carries the probe result and the extraction rate under `video`. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + registered = SourceService(workspace).list(resolved.id) + return wire.page([wire.source(s) for s in registered]) + + +def backfill_thumbnails(project: ProjectRef) -> dict[str, Any]: + """Render the previews that are missing for a project's assets. + + `get_asset_image` serves a cached preview and refuses rather than rendering + one on demand; this is the tool that refusal names. Ingest caches a preview + for everything it writes, so a missing one means an asset that predates the + cache or whose bytes would not render. + + Idempotent — assets that already have one are not re-rendered. `missing` and + `unreadable` are different damage: the first is a content blob that is gone, + which no preview pass can repair, and the second is bytes that are present + and will not decode. Neither is a failure of this call. + """ + with opened_workspace() as workspace: + resolved = resolve_project(workspace, project) + report = IngestService(workspace).backfill_thumbnails(resolved.id) + return wire.thumbnail_backfill(report) diff --git a/tests/cli/test_mcp_command.py b/tests/cli/test_mcp_command.py new file mode 100644 index 00000000..0adb7532 --- /dev/null +++ b/tests/cli/test_mcp_command.py @@ -0,0 +1,182 @@ +"""``visionset mcp`` — resolving the workspace, then handing stdio to the child. + +The command itself is four lines of real work, and all four are worth pinning: +the workspace is resolved with the full precedence, stated in the environment +*before* the child starts, refused at a terminal when it is not a workspace, and +the child is spawned rather than imported. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path +from typing import Any + +import pytest +from typer.testing import CliRunner + +from visionset.cli import mcp as mcp_module +from visionset.cli.main import app +from visionset.kernel.services import WORKSPACE_ENV_VAR, WorkspaceService + +runner = CliRunner() + + +class Spawn: + """Records the one subprocess call, with the environment as it stood *inside* it. + + Snapshotting ``os.environ`` here rather than after the command returns is the + whole point: asserting afterwards would only prove the variable was set at + some point, and the claim worth making is that it was set before the server + started. + """ + + def __init__(self, returncode: int = 0) -> None: + self.returncode = returncode + self.argv: list[str] | None = None + self.env: dict[str, str] = {} + + def __call__(self, argv: list[str], **kwargs: Any) -> subprocess.CompletedProcess[bytes]: + self.argv = argv + self.env = dict(os.environ) + return subprocess.CompletedProcess(argv, self.returncode) + + +@pytest.fixture(autouse=True) +def _no_ambient_workspace(monkeypatch: pytest.MonkeyPatch) -> None: + # `setenv(..., "")`, never `delenv(..., raising=False)` — the latter records no + # undo when the variable was already absent, and this command *writes* the + # variable, so it would leak into every module collected after this one. An + # empty value is what `resolve_workspace_root` and a shell both read as unset. + monkeypatch.setenv(WORKSPACE_ENV_VAR, "") + + +@pytest.fixture +def spawn(monkeypatch: pytest.MonkeyPatch) -> Spawn: + recorder = Spawn() + # `subprocess.run` is patched directly: it is a documented public entry point, + # so no seam had to be invented to make this testable. + monkeypatch.setattr(subprocess, "run", recorder) + return recorder + + +def _workspace(tmp_path: Path) -> Path: + root = tmp_path / "ws" + WorkspaceService.init(root).close() + return root + + +def test_the_flag_names_the_workspace_and_the_child_is_told_before_it_starts( + tmp_path: Path, spawn: Spawn +) -> None: + root = _workspace(tmp_path) + result = runner.invoke(app, ["mcp", "--workspace", str(root)]) + assert result.exit_code == 0 + assert spawn.env[WORKSPACE_ENV_VAR] == str(root) + + +def test_the_child_is_this_interpreter_running_the_server_module( + tmp_path: Path, spawn: Spawn +) -> None: + # Named rather than imported: import-linter forbids `visionset.cli` importing + # `visionset.mcp`, and stdio has to be inherited by a real process anyway. + root = _workspace(tmp_path) + runner.invoke(app, ["mcp", "--workspace", str(root)]) + assert spawn.argv == [sys.executable, "-m", "visionset.mcp.main"] + + +def test_the_environment_variable_is_used_when_no_flag_is_given( + tmp_path: Path, spawn: Spawn, monkeypatch: pytest.MonkeyPatch +) -> None: + root = _workspace(tmp_path) + monkeypatch.setenv(WORKSPACE_ENV_VAR, str(root)) + assert runner.invoke(app, ["mcp"]).exit_code == 0 + assert spawn.env[WORKSPACE_ENV_VAR] == str(root) + + +def test_the_working_directory_is_walked_upward_when_nobody_said( + tmp_path: Path, spawn: Spawn, monkeypatch: pytest.MonkeyPatch +) -> None: + root = _workspace(tmp_path) + below = root / "deeper" / "still" + below.mkdir(parents=True) + monkeypatch.chdir(below) + assert runner.invoke(app, ["mcp"]).exit_code == 0 + # The command applies the full precedence and then *states* it, so the child's + # own resolver stops at branch 2 and the two cannot disagree. + assert spawn.env[WORKSPACE_ENV_VAR] == str(root) + + +def test_the_flag_pointed_below_a_workspace_does_not_walk_up_to_it( + tmp_path: Path, spawn: Spawn +) -> None: + # This branch's own walk-negative, the sibling of `visionset ui`'s and of the + # kernel's. A stated directory is somebody saying which workspace, and trading + # it for its parent is how an agent is pointed at the wrong one. + root = _workspace(tmp_path) + below = root / "deeper" + below.mkdir() + result = runner.invoke(app, ["mcp", "--workspace", str(below)]) + assert result.exit_code == 1 + assert spawn.argv is None, "nothing should have been spawned" + + +def test_a_directory_that_is_not_a_workspace_is_refused_before_anything_spawns( + tmp_path: Path, spawn: Spawn +) -> None: + # The pre-flight open is real, not a check: a refusal here is one sentence at + # exit 1, where inside the child it would be a JSON envelope nobody watches. + result = runner.invoke(app, ["mcp", "--workspace", str(tmp_path)]) + assert result.exit_code == 1 + assert "Error:" in result.stderr + assert spawn.argv is None + + +def test_the_refusal_names_a_remedy_a_person_at_a_terminal_can_use( + tmp_path: Path, spawn: Spawn +) -> None: + result = runner.invoke(app, ["mcp", "--workspace", str(tmp_path)]) + assert "--workspace" in result.stderr or WORKSPACE_ENV_VAR in result.stderr + + +def test_the_banner_goes_to_stderr_because_stdout_is_the_protocol( + tmp_path: Path, spawn: Spawn +) -> None: + # A single stray line on stdout would corrupt the JSON-RPC stream before the + # first message. This is the assertion that keeps it that way. + root = _workspace(tmp_path) + result = runner.invoke(app, ["mcp", "--workspace", str(root)]) + assert result.stdout == "" + assert str(root) in result.stderr + + +def test_the_childs_exit_code_is_this_commands_exit_code( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + root = _workspace(tmp_path) + monkeypatch.setattr(subprocess, "run", Spawn(returncode=3)) + assert runner.invoke(app, ["mcp", "--workspace", str(root)]).exit_code == 3 + + +def test_no_workspace_sidecar_is_left_behind_for_the_child_to_recover( + tmp_path: Path, spawn: Spawn +) -> None: + # The pre-flight open closes again, which checkpoints the WAL. The child is + # about to open the same file. + root = _workspace(tmp_path) + runner.invoke(app, ["mcp", "--workspace", str(root)]) + assert not (root / "visionset.db-wal").exists() + + +def test_the_command_is_listed_in_the_help() -> None: + assert "mcp" in runner.invoke(app, ["--help"]).stdout + + +def test_the_cli_does_not_import_the_mcp_package() -> None: + # The independence contract in prose, asserted where a reader will see it. + # import-linter enforces it in CI; this fails at the point of the mistake. + source = Path(mcp_module.__file__).read_text() + assert "import visionset.mcp" not in source + assert "from visionset.mcp" not in source diff --git a/tests/mcp/_flow.py b/tests/mcp/_flow.py new file mode 100644 index 00000000..36b255cc --- /dev/null +++ b/tests/mcp/_flow.py @@ -0,0 +1,156 @@ +# usage: from tests.mcp._flow import call, ok, payload, workspace +"""Calling tools the way an MCP client does, and walking the cycle up to a rung. + +Plain functions, the way ``tests/cli/_flow.py`` and ``tests/server/_flow.py`` are +plain functions — there is no ``conftest.py`` anywhere in this repository and this +is not the module that starts one. + +**Every call goes through the real protocol.** ``create_connected_server_and_client_session`` +runs the actual server over a pair of in-memory streams and hands back a real +``ClientSession``, so a test sees what a client sees: a ``CallToolResult`` with +``isError`` and ``structuredContent``, and — for any tool that declares an output +schema — free validation of the result against it on every call. +``FastMCP.call_tool`` is deliberately **not** used: it skips input validation and +output validation, returns an undocumented two-tuple, and raises where the +protocol returns ``isError``. + +**No async test infrastructure.** :func:`call` bridges with ``anyio.run``, so +every test module here is plain synchronous pytest with no marker, no fixture and +no plugin. A fresh session per call is not waste — the server opens and closes the +workspace per tool call anyway, so it is what production does too. + +**Every rung is reached by calling tools**, never by reaching past them into the +SDK. A helper that built "an approved batch" out of ``BatchService`` would test +the later tool against a state no agent can produce. Reading state *back* for an +assertion goes through the kernel, because the tool's answer is what is under +test and cannot also be the evidence. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import anyio +from mcp.shared.memory import create_connected_server_and_client_session +from mcp.types import CallToolResult +from tests.fixtures.media import write_images + +from visionset.kernel.services import WorkspaceService +from visionset.mcp.main import server + +SCHEMA_CLASSES: list[dict[str, Any]] = [ + { + "name": "sign", + "geometry": "bbox", + "color": "#ff0000", + "attributes": [{"name": "occluded", "kind": "boolean", "default": False}], + } +] +"""The smallest schema that is not trivial: one class, one optional attribute.""" + +BBOX: dict[str, Any] = {"type": "bbox", "x": 1.0, "y": 2.0, "width": 8.0, "height": 6.0} +"""A box that fits inside the fixtures' 64x48 images. ``type`` is always spelled out.""" + + +def call(tool: str, /, **arguments: Any) -> CallToolResult: + """Invoke one tool over a real client session and return the whole result.""" + + async def go() -> CallToolResult: + async with create_connected_server_and_client_session(server) as client: + return await client.call_tool(tool, arguments) + + return anyio.run(go) + + +def tool_names() -> list[str]: + """Every tool the server advertises, in registration order.""" + + async def go() -> list[str]: + async with create_connected_server_and_client_session(server) as client: + return [t.name for t in (await client.list_tools()).tools] + + return anyio.run(go) + + +def tool_schemas() -> dict[str, Any]: + """Every advertised tool, keyed by name, for assertions about the listing itself.""" + + async def go() -> dict[str, Any]: + async with create_connected_server_and_client_session(server) as client: + return {t.name: t for t in (await client.list_tools()).tools} + + return anyio.run(go) + + +def payload(result: CallToolResult) -> dict[str, Any]: + """The structured half of a successful call, asserted to be one. + + ``isError`` covers a malformed *request* — an argument pydantic refused before + the body ran. A domain refusal is a perfectly ordinary result carrying the + error envelope, which is what :func:`error` is for. + """ + assert not result.isError, result.content + assert result.structuredContent is not None + assert "error" not in result.structuredContent, result.structuredContent + return result.structuredContent + + +def error(result: CallToolResult) -> dict[str, Any]: + """The error envelope of a refused call, asserted to be one.""" + assert not result.isError, "a domain refusal is a result, not a protocol error" + assert result.structuredContent is not None + assert "error" in result.structuredContent, result.structuredContent + envelope: dict[str, Any] = result.structuredContent["error"] + return envelope + + +def workspace(monkeypatch: Any, tmp_path: Path) -> Path: + """Create a workspace and point the server at it through the environment. + + ``setenv`` to the empty string is how a test says "no ambient workspace" — + never ``delenv(..., raising=False)``, which records no undo when the variable + was already absent and would leak a developer's own workspace into the suite. + """ + root = tmp_path / "ws" + WorkspaceService.init(root).close() + monkeypatch.setenv("VISIONSET_WORKSPACE", str(root)) + return root + + +def project(monkeypatch: Any, tmp_path: Path, *, name: str = "road-signs") -> str: + """A workspace with one project in it. Returns the project name.""" + workspace(monkeypatch, tmp_path) + payload(call("create_project", name=name)) + return name + + +def schema(monkeypatch: Any, tmp_path: Path, *, name: str = "road-signs") -> str: + """A project with schema version 1.""" + named = project(monkeypatch, tmp_path, name=name) + payload(call("create_schema_version", project=named, classes=SCHEMA_CLASSES)) + return named + + +def ingested( + monkeypatch: Any, tmp_path: Path, *, count: int = 2, name: str = "road-signs" +) -> tuple[str, str]: + """A project with a schema and one batch of freshly ingested stills. + + Returns ``(project_name, batch_id)``. + """ + named = schema(monkeypatch, tmp_path, name=name) + incoming = tmp_path / "incoming" + write_images(incoming, count=count) + result = payload(call("ingest", project=named, path=str(incoming))) + return named, str(result["batch_id"]) + + +def open_batch( + monkeypatch: Any, tmp_path: Path, *, count: int = 2, name: str = "road-signs" +) -> tuple[str, str, str]: + """A batch approved and started, with its single job. Returns ``(project, batch, job)``.""" + named, batch_id = ingested(monkeypatch, tmp_path, count=count, name=name) + payload(call("approve_batch", batch_id=batch_id)) + started = payload(call("start_batch", batch_id=batch_id)) + return named, batch_id, str(started["jobs"][0]["id"]) diff --git a/tests/mcp/test_agent_walk.py b/tests/mcp/test_agent_walk.py new file mode 100644 index 00000000..18543a6d --- /dev/null +++ b/tests/mcp/test_agent_walk.py @@ -0,0 +1,179 @@ +"""The whole cycle, driven by an agent, in one function. + +The `tests/server/test_external_client.py` precedent, for the same reason: the +point is that the *entire* walk is visible at once, so this module deliberately +uses **none** of `_flow.py`'s ladder helpers. Every call's outcome is asserted +rather than only the final state, because a walk that quietly skipped a step and +still ended up in the right place would prove nothing. + +It also stands in for #36, which owns the published transcript: if this passes, an +agent holding nothing but a workspace can produce a released, verified, exported +dataset — including looking at the pixels before labelling them. +""" + +from __future__ import annotations + +import base64 +import io +from pathlib import Path +from typing import Any + +import pytest +from mcp.types import CallToolResult +from PIL import Image as PillowImage +from tests.fixtures.media import write_images +from tests.mcp._flow import call + +from visionset.kernel.services import WorkspaceService + + +def ok(result: CallToolResult) -> dict[str, Any]: + """Every step asserts, and an error envelope is a failure here.""" + assert not result.isError, result.content + assert result.structuredContent is not None + assert "error" not in result.structuredContent, result.structuredContent + return result.structuredContent + + +def test_an_agent_can_take_a_folder_of_images_to_an_exported_release( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + root = tmp_path / "ws" + WorkspaceService.init(root).close() + monkeypatch.setenv("VISIONSET_WORKSPACE", str(root)) + write_images(tmp_path / "incoming", count=4, size=(640, 480)) + + # 1. Discover an empty workspace and make somewhere to work. + assert ok(call("list_projects")) == {"items": [], "total": 0} + created = ok(call("create_project", name="road-signs", description="signage survey")) + assert created["project"]["name"] == "road-signs" + + # 2. Declare the contract before any work is judged against it. + schema = ok( + call( + "create_schema_version", + project="road-signs", + classes=[ + {"name": "sign", "geometry": "bbox"}, + {"name": "empty-road", "geometry": "classification_tag"}, + ], + ) + ) + assert schema["version"] == 1 + assert ok(call("get_schema", project="road-signs"))["active_version"] == 1 + + # 3. Read the folder in. One call, synchronous, and the batch comes back. + run = ok(call("ingest", project="road-signs", path=str(tmp_path / "incoming"))) + assert (run["created"], run["deduplicated"], run["failed"]) == (4, 0, 0) + batch_id = run["batch_id"] + + # 4. Freeze it, pin the schema, cut it in two, and open it for work. + approved = ok(call("approve_batch", batch_id=batch_id, jobs_of=2)) + assert approved["schema_version"] == 1 + assert len(approved["jobs"]) == 2 + started = ok(call("start_batch", batch_id=batch_id)) + assert started["state"] == "in_annotation" + + # 5. Work each job: look, then label. This is the part that makes it an + # annotator rather than an operator. + for job in started["jobs"]: + job_id = job["id"] + assert ok(call("start_job", job_id=job_id))["state"] == "in_progress" + pending = ok(call("next_pending_assets", job_id=job_id, count=10)) + assert pending["total"] == 2 + + for index, asset in enumerate(pending["items"]): + seen = call("get_asset_image", project="road-signs", asset_id=asset["id"]) + frame = ok(seen) + block = next(b for b in seen.content if b.type == "image") + with PillowImage.open(io.BytesIO(base64.b64decode(block.data))) as opened: + assert opened.size == (frame["image_width"], frame["image_height"]) + + # A 640x480 asset previews at 256x192, so the agent measured on a + # frame 2.5x smaller than the one coordinates live in. Scaling is not + # optional, and nothing downstream could detect the omission. + assert (frame["width"], frame["height"]) == (640, 480) + assert frame["scale"] == pytest.approx(640 / frame["image_width"]) + assert frame["scale"] > 1 + + if index == 0: + measured = {"x": 10.0, "y": 12.0, "width": 40.0, "height": 30.0} + scaled = {k: v * frame["scale"] for k, v in measured.items()} + written = ok( + call( + "add_annotations", + job_id=job_id, + annotations=[ + { + "asset_id": asset["id"], + "label_class": "sign", + "geometry": {"type": "bbox", **scaled}, + "provenance": "model", + "model_ref": "walkthrough@1", + "confidence": 0.82, + } + ], + ) + ) + assert written["items"][0]["geometry"]["width"] == pytest.approx( + 40.0 * frame["scale"] + ) + # In the asset's own frame, not the preview's — the box has to fit + # the real image, which is the whole reason `scale` is published. + assert written["items"][0]["geometry"]["x"] < frame["width"] + else: + ok( + call( + "set_asset_progress", + job_id=job_id, + asset_id=asset["id"], + progress="skipped", + ) + ) + + assert ok(call("next_pending_assets", job_id=job_id, count=10))["total"] == 0 + assert ok(call("complete_job", job_id=job_id))["state"] == "completed" + + # 6. Close the batch and move the finished work into the trunk. The two + # skipped assets stay behind, which is what a skip is for. + assert ok(call("complete_batch", batch_id=batch_id))["state"] == "completed" + assert ok(call("promote_batch", batch_id=batch_id))["total"] == 2 + + stats = ok(call("dataset_stats", project="road-signs")) + assert (stats["asset_count"], stats["annotation_count"]) == (2, 2) + assert stats["classes"] == [{"label_class": "sign", "annotations": 2, "assets": 2}] + # A declared class nobody used is absent, not zero. + assert "empty-road" not in {c["label_class"] for c in stats["classes"]} + + # 7. Freeze, check, and write it where something can train on it. + release = ok( + call( + "publish_release", + project="road-signs", + tag="v1.0", + split={"train": 0.5, "val": 0.25, "test": 0.25, "seed": 7}, + ) + ) + assert (release["asset_count"], release["annotation_count"]) == (2, 2) + assert ok(call("list_releases", project="road-signs"))["total"] == 1 + assert ok(call("verify_release", project="road-signs", tag="v1.0"))["ok"] is True + + assert ok(call("list_formats"))["items"] == [{"name": "dummy", "lossy": False}] + exported = ok( + call( + "export_release", + project="road-signs", + tag="v1.0", + format="dummy", + dest=str(tmp_path / "out"), + ) + ) + assert exported["release_id"] == release["id"] + assert Path(exported["directory"]).is_dir() + + # 8. And the walk ends on a refusal it also asserts: the release is immutable, + # so the tag cannot be reused. + reused = call("publish_release", project="road-signs", tag="v1.0") + assert not reused.isError + assert reused.structuredContent is not None + assert "error" in reused.structuredContent diff --git a/tests/mcp/test_annotation_tools.py b/tests/mcp/test_annotation_tools.py new file mode 100644 index 00000000..2c2b4736 --- /dev/null +++ b/tests/mcp/test_annotation_tools.py @@ -0,0 +1,219 @@ +"""The three annotation writes and the read that supplies their ids. + +The two things worth pinning hardest: every write is all-or-nothing, and when one +item is bad the refusal carries the position in the list the caller sent — which +is recoverable nowhere else, because nothing landed to count from. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any +from uuid import uuid4 + +import pytest +from tests.mcp._flow import BBOX, call, error, open_batch, payload + + +def _label(asset_id: str, **overrides: Any) -> dict[str, Any]: + body: dict[str, Any] = { + "asset_id": asset_id, + "label_class": "sign", + "geometry": BBOX, + "provenance": "model", + "model_ref": "probe@1", + "confidence": 0.9, + } + return {**body, **overrides} + + +def _job_with_assets( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, *, count: int = 2 +) -> tuple[str, list[str]]: + _, _, job_id = open_batch(monkeypatch, tmp_path, count=count) + payload(call("start_job", job_id=job_id)) + assets = payload(call("next_pending_assets", job_id=job_id, count=count))["items"] + return job_id, [a["id"] for a in assets] + + +def test_a_written_annotation_comes_back_with_the_pinned_version_stamped_in( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `schema_version` is not an input — the service stamps the batch's pin over + # whatever it was handed, which is why `AnnotationInput` omits the field. + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + written = payload(call("add_annotations", job_id=job_id, annotations=[_label(assets[0])])) + assert written["total"] == 1 + assert written["items"][0]["schema_version"] == 1 + assert written["items"][0]["provenance"] == "model" + assert written["items"][0]["model_ref"] == "probe@1" + assert written["items"][0]["geometry"] == BBOX + + +def test_an_asset_with_nothing_on_it_lists_an_empty_collection( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + assert payload(call("list_asset_annotations", job_id=job_id, asset_id=assets[0])) == { + "items": [], + "total": 0, + } + + +def test_a_bad_item_refuses_the_whole_write_and_names_its_position( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + refusal = error( + call( + "add_annotations", + job_id=job_id, + annotations=[ + _label(assets[0]), + _label(assets[1]), + _label(assets[0], label_class="pedestrian"), + ], + ) + ) + assert refusal["index"] == 2 + # All-or-nothing: the two good ones did not land either, which is exactly why + # the index is the only thing identifying the offending item. + assert payload(call("list_asset_annotations", job_id=job_id, asset_id=assets[0]))["total"] == 0 + + +def test_the_geometry_must_match_the_one_its_class_is_bound_to( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + refusal = error( + call( + "add_annotations", + job_id=job_id, + annotations=[ + _label( + assets[0], + geometry={"type": "polygon", "points": [[0, 0], [4, 0], [4, 4]]}, + ) + ], + ) + ) + assert refusal["index"] == 0 + + +def test_an_attribute_the_class_does_not_declare_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + assert error( + call( + "add_annotations", + job_id=job_id, + annotations=[_label(assets[0], attributes={"colour": "red"})], + ) + )["message"] + + +def test_provenance_model_without_a_reference_is_a_malformed_request( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # A domain model validator on `AnnotationInput`'s own fields, so it fires + # during argument parsing rather than reaching the service. + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + assert call( + "add_annotations", + job_id=job_id, + annotations=[{**_label(assets[0]), "model_ref": None}], + ).isError + + +def test_a_geometry_with_no_type_cannot_pick_a_variant( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The wart the domain union brings with it, pinned rather than hidden: the + # discriminator carries a default, so the generated schema shows `type` as + # optional, but pydantic reads the tag out of the input dict to select the + # variant and fails without it. The tool description says to spell it out. + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + result = call( + "add_annotations", + job_id=job_id, + annotations=[_label(assets[0], geometry={"x": 1.0, "y": 2.0, "width": 3.0, "height": 4.0})], + ) + assert result.isError + assert "union_tag_not_found" in result.content[0].text or "type" in result.content[0].text + + +def test_an_update_replaces_the_whole_value_and_keeps_the_stored_asset( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + written = payload(call("add_annotations", job_id=job_id, annotations=[_label(assets[0])])) + annotation_id = written["items"][0]["id"] + moved = {"type": "bbox", "x": 5.0, "y": 5.0, "width": 2.0, "height": 2.0} + updated = payload( + call( + "update_annotations", + job_id=job_id, + annotations=[ + { + "id": annotation_id, + "label_class": "sign", + "geometry": moved, + "provenance": "human", + } + ], + ) + ) + assert updated["items"][0]["geometry"] == moved + assert updated["items"][0]["provenance"] == "human" + # The stored asset always wins — there is no way to move a label to another + # image, which is why `AnnotationEdit` has no `asset_id` at all. + assert updated["items"][0]["asset_id"] == assets[0] + + +def test_deleting_takes_no_confirmation_and_moves_the_asset_back( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # One of exactly two kernel methods exempt from `confirm`: removing a label is + # the annotator edit loop, and the batch gate is the guard. + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + written = payload(call("add_annotations", job_id=job_id, annotations=[_label(assets[0])])) + assert payload( + call("delete_annotations", job_id=job_id, annotation_ids=[written["items"][0]["id"]]) + ) == {"deleted": 1} + assert payload(call("get_job", job_id=job_id))["progress"]["unannotated"] == 2 + + +def test_a_repeated_id_counts_once_and_an_unknown_one_blames_its_own_position( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + job_id, assets = _job_with_assets(monkeypatch, tmp_path) + written = payload( + call("add_annotations", job_id=job_id, annotations=[_label(assets[0]), _label(assets[1])]) + ) + first, second = (a["id"] for a in written["items"]) + assert payload( + call("delete_annotations", job_id=job_id, annotation_ids=[first, first, second]) + ) == {"deleted": 2} + + written = payload(call("add_annotations", job_id=job_id, annotations=[_label(assets[0])])) + stranger = str(uuid4()) + refusal = error( + call( + "delete_annotations", + job_id=job_id, + annotation_ids=[written["items"][0]["id"], stranger], + ) + ) + assert refusal["index"] == 1 + + +def test_no_write_reaches_a_batch_that_is_not_open( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + job_id, assets = _job_with_assets(monkeypatch, tmp_path, count=1) + payload(call("set_asset_progress", job_id=job_id, asset_id=assets[0], progress="skipped")) + payload(call("complete_job", job_id=job_id)) + batch_id = payload(call("get_job", job_id=job_id))["batch_id"] + payload(call("complete_batch", batch_id=batch_id)) + assert error(call("add_annotations", job_id=job_id, annotations=[_label(assets[0])]))["message"] diff --git a/tests/mcp/test_asset_tools.py b/tests/mcp/test_asset_tools.py new file mode 100644 index 00000000..e437ea70 --- /dev/null +++ b/tests/mcp/test_asset_tools.py @@ -0,0 +1,177 @@ +"""``get_asset_image`` — the acceptance walk for "an agent can see what it annotates". + +The claim under test is not "bytes came back" but "the numbers beside the bytes +say which frame to write coordinates in". A preview whose ``scale`` were wrong +would produce annotations that are individually plausible and uniformly wrong. +""" + +from __future__ import annotations + +import base64 +import io +from pathlib import Path + +import pytest +from PIL import Image as PillowImage +from tests.fixtures.media import write_images +from tests.mcp._flow import call, error, ingested, payload, schema + +from visionset.kernel.domain import ImageFormat +from visionset.kernel.ports import DEFAULT_THUMBNAIL_MAX_EDGE, THUMBNAIL_FORMAT +from visionset.mcp.assets import SUFFIXES + + +def _image_block(result: object) -> object: + blocks = [b for b in result.content if b.type == "image"] # type: ignore[attr-defined] + assert len(blocks) == 1, result.content # type: ignore[attr-defined] + return blocks[0] + + +def _first_asset(named: str, batch_id: str) -> str: + listed = payload(call("list_batch_assets", batch_id=batch_id)) + return str(listed["items"][0]["id"]) + + +def test_a_preview_comes_back_as_image_content_with_the_rows_own_dimensions( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named, batch_id = ingested(monkeypatch, tmp_path, count=1) + asset_id = _first_asset(named, batch_id) + listed = payload(call("list_batch_assets", batch_id=batch_id))["items"][0] + + result = call("get_asset_image", project=named, asset_id=asset_id) + assert not result.isError + block = _image_block(result) + assert block.mimeType == "image/jpeg" # type: ignore[attr-defined] + + meta = result.structuredContent + assert meta is not None + # The acceptance criterion: the dimensions the tool reports match the Asset row. + assert (meta["width"], meta["height"]) == (listed["width"], listed["height"]) + assert meta["content_hash"] == listed["content_hash"] + assert meta["resolution"] == "thumbnail" + + +def test_the_bytes_decode_to_an_image_of_the_size_the_answer_claims( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # Measured, never derived: the port caps an *edge*, so the preview's size is a + # function of the aspect ratio and of whether it was smaller to begin with. + named, batch_id = ingested(monkeypatch, tmp_path, count=1) + result = call("get_asset_image", project=named, asset_id=_first_asset(named, batch_id)) + block = _image_block(result) + with PillowImage.open(io.BytesIO(base64.b64decode(block.data))) as opened: # type: ignore[attr-defined] + assert opened.size == ( + result.structuredContent["image_width"], # type: ignore[index] + result.structuredContent["image_height"], # type: ignore[index] + ) + assert opened.format == "JPEG" + + +def test_a_large_asset_is_previewed_smaller_and_the_scale_says_by_how_much( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The whole reason four numbers travel. An agent measuring a box on the + # returned pixels has to multiply by `scale` to reach the frame annotations + # live in, and nothing downstream could detect the mistake if it did not. + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "big", count=1, size=(1024, 512)) + batch_id = payload(call("ingest", project=named, path=str(tmp_path / "big")))["batch_id"] + + meta = call( + "get_asset_image", project=named, asset_id=_first_asset(named, batch_id) + ).structuredContent + assert meta is not None + assert (meta["width"], meta["height"]) == (1024, 512) + assert max(meta["image_width"], meta["image_height"]) == DEFAULT_THUMBNAIL_MAX_EDGE + assert meta["scale"] == pytest.approx(1024 / meta["image_width"]) + assert meta["scale"] > 1 + + +def test_an_asset_smaller_than_the_cap_is_never_enlarged_and_scales_by_one( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named, batch_id = ingested(monkeypatch, tmp_path, count=1) + meta = call( + "get_asset_image", project=named, asset_id=_first_asset(named, batch_id) + ).structuredContent + assert meta is not None + assert (meta["image_width"], meta["image_height"]) == (meta["width"], meta["height"]) + assert meta["scale"] == 1.0 + + +def test_asking_for_full_resolution_returns_the_original_bytes( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "big", count=1, size=(400, 300)) + batch_id = payload(call("ingest", project=named, path=str(tmp_path / "big")))["batch_id"] + asset_id = _first_asset(named, batch_id) + + result = call("get_asset_image", project=named, asset_id=asset_id, full=True) + block = _image_block(result) + assert block.mimeType == "image/png" # type: ignore[attr-defined] + meta = result.structuredContent + assert meta is not None + assert meta["resolution"] == "full" + assert (meta["image_width"], meta["image_height"]) == (400, 300) + assert meta["scale"] == 1.0 + with PillowImage.open(io.BytesIO(base64.b64decode(block.data))) as opened: # type: ignore[attr-defined] + assert opened.size == (400, 300) + + +def test_a_missing_preview_names_the_tool_that_renders_one( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + from visionset.kernel.services import ProjectService, WorkspaceService + + root = tmp_path / "ws" + named, batch_id = ingested(monkeypatch, tmp_path, count=1) + asset_id = _first_asset(named, batch_id) + with WorkspaceService.open(root) as service: + project_id = ProjectService(service).get_by_name(named).id + with service.unit_of_work() as uow: + asset = uow.assets.list(project_id)[0] + uow.assets.update(asset.model_copy(update={"thumbnail_hash": None})) + + refusal = error(call("get_asset_image", project=named, asset_id=asset_id)) + assert "backfill_thumbnails" in (refusal["hint"] or "") + assert "full=true" in (refusal["hint"] or "") + # And the remedy it names actually works. + payload(call("backfill_thumbnails", project=named)) + assert not call("get_asset_image", project=named, asset_id=asset_id).isError + + +def test_an_unknown_asset_is_refused_in_the_envelope( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + from uuid import uuid4 + + named, _ = ingested(monkeypatch, tmp_path, count=1) + assert error(call("get_asset_image", project=named, asset_id=str(uuid4())))["message"] + + +def test_a_malformed_asset_id_is_refused_before_the_kernel_sees_it( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named, _ = ingested(monkeypatch, tmp_path, count=1) + assert ( + "must be a UUID" + in error(call("get_asset_image", project=named, asset_id="not-a-uuid"))["message"] + ) + + +def test_every_image_format_has_a_media_type() -> None: + # The `ProgressCounts` bargain: a format added to the enum without an entry + # fails here rather than crashing one download in a thousand, or serving the + # bytes under the wrong type. + assert set(SUFFIXES) == set(ImageFormat) + assert SUFFIXES[THUMBNAIL_FORMAT] == "jpeg" + + +def test_the_description_states_the_cap_the_port_actually_pins() -> None: + # The number is spelled out in the docstring because a docstring cannot be an + # f-string. This is the tripwire that keeps the two in step. + from visionset.mcp.assets import get_asset_image + + assert str(DEFAULT_THUMBNAIL_MAX_EDGE) in (get_asset_image.__doc__ or "") diff --git a/tests/mcp/test_batch_tools.py b/tests/mcp/test_batch_tools.py new file mode 100644 index 00000000..a915e99f --- /dev/null +++ b/tests/mcp/test_batch_tools.py @@ -0,0 +1,180 @@ +"""The batch lifecycle, the two listings, and promotion into the trunk.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from tests.mcp._flow import BBOX, call, error, ingested, open_batch, payload, schema + + +def test_a_freshly_ingested_batch_is_a_draft_with_no_jobs_and_no_pin( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=2) + batch = payload(call("get_batch", batch_id=batch_id)) + assert batch["state"] == "draft" + assert batch["schema_version"] is None + assert batch["jobs"] == [] + assert batch["asset_count"] == 2 + + +def test_approval_pins_the_active_schema_and_cuts_one_job_by_default( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=4) + approved = payload(call("approve_batch", batch_id=batch_id)) + assert approved["state"] == "approved" + assert approved["schema_version"] == 1 + assert len(approved["jobs"]) == 1 + assert approved["jobs"][0]["asset_count"] == 4 + + +def test_jobs_of_cuts_the_batch_into_segments( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=5) + approved = payload(call("approve_batch", batch_id=batch_id, jobs_of=2)) + # An exact partition: 5 assets in jobs of 2 is 2 + 2 + 1, never a dropped one. + assert sorted(j["asset_count"] for j in approved["jobs"]) == [1, 2, 2] + + +def test_jobs_of_zero_is_a_malformed_request_rather_than_a_domain_refusal( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `BySize.size` is `gt=0` and constructing one with zero raises a pydantic + # ValidationError, which is not a VisionSetError. `ge=1` on the parameter is + # what stops it ever being constructed. + _, batch_id = ingested(monkeypatch, tmp_path, count=2) + result = call("approve_batch", batch_id=batch_id, jobs_of=0) + assert result.isError + + +def test_the_lifecycle_is_one_way(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=1) + payload(call("approve_batch", batch_id=batch_id)) + assert "cannot become" in error(call("approve_batch", batch_id=batch_id))["message"] + payload(call("start_batch", batch_id=batch_id)) + assert "cannot become" in error(call("start_batch", batch_id=batch_id))["message"] + + +def test_starting_a_batch_that_was_never_approved_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=1) + assert "cannot become" in error(call("start_batch", batch_id=batch_id))["message"] + + +def test_approving_an_empty_batch_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + from tests.fixtures.media import write_unsupported_file + + named = schema(monkeypatch, tmp_path) + (tmp_path / "incoming").mkdir() + write_unsupported_file(tmp_path / "incoming" / "notes.txt") + result = payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + assert error(call("approve_batch", batch_id=result["batch_id"]))["message"] + + +def test_a_batch_cannot_be_completed_while_a_job_is_outstanding( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # Derived means recomputed, not automatic. + _, batch_id, _ = open_batch(monkeypatch, tmp_path, count=2) + assert error(call("complete_batch", batch_id=batch_id))["message"] + + +def test_listing_batch_assets_names_the_job_each_belongs_to( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id, job_id = open_batch(monkeypatch, tmp_path, count=3) + listed = payload(call("list_batch_assets", batch_id=batch_id)) + assert listed["total"] == 3 + assert {a["job_id"] for a in listed["items"]} == {job_id} + assert {a["progress"] for a in listed["items"]} == {"unannotated"} + + +def test_a_draft_batch_lists_its_assets_with_no_job_and_no_progress( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # Both null exactly while the batch is a draft, which is honest rather than + # lossy: a draft genuinely has no jobs to belong to. + _, batch_id = ingested(monkeypatch, tmp_path, count=2) + listed = payload(call("list_batch_assets", batch_id=batch_id)) + assert [a["job_id"] for a in listed["items"]] == [None, None] + assert [a["progress"] for a in listed["items"]] == [None, None] + + +def test_paging_bounds_the_response_and_leaves_the_total_alone( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=5) + first = payload(call("list_batch_assets", batch_id=batch_id, limit=2)) + second = payload(call("list_batch_assets", batch_id=batch_id, limit=2, offset=2)) + assert len(first["items"]) == 2 + assert len(second["items"]) == 2 + # `total` is the size of the whole batch, so a caller pages until it has seen + # that many rather than until the number moves. + assert first["total"] == second["total"] == 5 + assert {a["id"] for a in first["items"]}.isdisjoint({a["id"] for a in second["items"]}) + + +def test_an_offset_past_the_end_is_an_empty_page_and_not_a_refusal( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=2) + assert payload(call("list_batch_assets", batch_id=batch_id, offset=99)) == { + "items": [], + "total": 2, + } + + +def test_promotion_carries_only_the_settled_assets_and_leaves_a_skip_behind( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id, job_id = open_batch(monkeypatch, tmp_path, count=2) + payload(call("start_job", job_id=job_id)) + pending = payload(call("next_pending_assets", job_id=job_id, count=2))["items"] + payload( + call( + "add_annotations", + job_id=job_id, + annotations=[ + { + "asset_id": pending[0]["id"], + "label_class": "sign", + "geometry": BBOX, + "provenance": "model", + "model_ref": "probe@1", + } + ], + ) + ) + payload( + call("set_asset_progress", job_id=job_id, asset_id=pending[1]["id"], progress="skipped") + ) + payload(call("complete_job", job_id=job_id)) + payload(call("complete_batch", batch_id=batch_id)) + + promoted = payload(call("promote_batch", batch_id=batch_id)) + assert [a["id"] for a in promoted["items"]] == [pending[0]["id"]] + # A union against what is there, so the second call adds nothing. + assert payload(call("promote_batch", batch_id=batch_id)) == {"items": [], "total": 0} + + +def test_promoting_an_incomplete_batch_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id, _ = open_batch(monkeypatch, tmp_path, count=1) + assert error(call("promote_batch", batch_id=batch_id))["message"] + + +def test_a_malformed_batch_id_is_refused_before_the_kernel_sees_it( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The same call the API makes: a value that could not have named anything is + # a malformed request, not a missing resource. + ingested(monkeypatch, tmp_path, count=1) + refusal = error(call("get_batch", batch_id="not-a-uuid")) + assert "must be a UUID" in refusal["message"] diff --git a/tests/mcp/test_ingest_tools.py b/tests/mcp/test_ingest_tools.py new file mode 100644 index 00000000..be5011eb --- /dev/null +++ b/tests/mcp/test_ingest_tools.py @@ -0,0 +1,172 @@ +"""``ingest`` / ``list_sources`` / ``backfill_thumbnails``. + +``ingest`` is the one tool standing for three parity candidates, so what is +pinned here is chiefly that the dispatch and the refusals happen at this level +rather than reaching the kernel as tracebacks. +""" + +from __future__ import annotations + +from pathlib import Path +from uuid import UUID + +import pytest +from tests.fixtures.media import ( + require_ffmpeg, + write_images, + write_unsupported_file, + write_video, +) +from tests.mcp._flow import call, error, payload, schema + +from visionset.kernel.services import IngestService, ProjectService, WorkspaceService + + +def test_a_directory_of_stills_becomes_one_batch( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=3) + result = payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + assert result["created"] == 3 + assert result["deduplicated"] == 0 + assert result["failed"] == 0 + assert result["source"]["kind"] == "image_directory" + assert result["batch_id"] + + +def test_ingesting_the_same_directory_again_creates_nothing_new( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # Registration is idempotent and content addressing does the rest, which is + # exactly why there is no `resume_ingest` tool: re-running is the remedy. + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=2) + first = payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + second = payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + assert second["created"] == 0 + assert second["deduplicated"] == 2 + assert second["source"]["id"] == first["source"]["id"] + + +def test_a_file_that_is_not_an_image_is_reported_and_the_run_carries_on( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=2) + write_unsupported_file(tmp_path / "incoming" / "notes.txt") + result = payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + assert result["created"] == 2 + assert len(result["failures"]) == 1 + # `IngestFailure.name` is whatever the run's own loop was holding, which for a + # directory walk is the full path rather than the basename. Worth knowing + # rather than worth changing: unlike `Source.path` and `Asset.uri`, which are + # deliberately unpublished, this one already travels on the wire through + # `IngestFailureOut` and is the same string the REST API and the CLI report. + assert result["failures"][0]["name"].endswith("notes.txt") + assert result["failures"][0]["kind"] == "unsupported" + assert "notes.txt" not in result["failures"][0]["reason"] + + +def test_a_missing_path_is_refused_before_any_work( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `canonical_path` resolves strictly and would raise FileNotFoundError, which + # is outside the VisionSetError tree and would reach the client as a + # traceback's text rather than as a refusal. + named = schema(monkeypatch, tmp_path) + refusal = error(call("ingest", project=named, path=str(tmp_path / "nowhere"))) + assert "no such path" in refusal["message"] + + +def test_a_non_positive_rate_is_refused_rather_than_raising( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `register_video` refuses this with a bare ValueError. Pydantic cannot express + # an exclusive lower bound through `Field(gt=...)` here without also rejecting + # the None default, so it is checked in the body — the CLI's `--fps` problem. + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=1) + refusal = error(call("ingest", project=named, path=str(tmp_path / "incoming"), fps=0)) + assert "greater than zero" in refusal["message"] + + +def test_a_rate_given_for_a_directory_of_stills_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=1) + refusal = error(call("ingest", project=named, path=str(tmp_path / "incoming"), fps=2.0)) + assert "directory of stills" in refusal["message"] + + +def test_the_batch_can_be_named(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=1) + result = payload( + call("ingest", project=named, path=str(tmp_path / "incoming"), batch_name="first pass") + ) + listed = payload(call("list_batches", project=named)) + assert [b["name"] for b in listed["items"]] == ["first pass"] + assert listed["items"][0]["id"] == result["batch_id"] + + +def test_a_clip_is_decomposed_into_frames(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + require_ffmpeg() + named = schema(monkeypatch, tmp_path) + clip = write_video(tmp_path / "clip.mp4", size=(96, 72)) + result = payload(call("ingest", project=named, path=str(clip.path), fps=1.0)) + assert result["source"]["kind"] == "video" + # The rate is part of what the source *is*, so it comes back on the source. + assert result["source"]["video"]["extraction_fps"] == 1.0 + assert result["created"] == 2 + + +def test_sources_are_listed_without_the_path_they_live_at( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=1) + payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + listed = payload(call("list_sources", project=named)) + assert listed["total"] == 1 + # `name` is the last component only. The absolute path describes this machine's + # disk and is not published anywhere. + assert listed["items"][0]["name"] == "incoming" + assert "path" not in listed["items"][0] + + +def test_backfill_reports_a_project_whose_previews_are_already_cached( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # Ingest caches a preview for everything it writes, so the ordinary answer is + # "nothing to do" — idempotent, and not an error. + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=2) + payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + report = payload(call("backfill_thumbnails", project=named)) + assert report["examined"] == 0 + assert report["filled"] == [] + + +def test_backfill_fills_a_preview_that_was_never_rendered( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + root = tmp_path / "ws" + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=1) + payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + # Clear the cached hash by hand — the state an asset written before the cache + # existed is in, which is the only thing this tool is for. + with WorkspaceService.open(root) as service: + project_id = ProjectService(service).get_by_name(named).id + with service.unit_of_work() as uow: + asset = uow.assets.list(project_id)[0] + uow.assets.update(asset.model_copy(update={"thumbnail_hash": None})) + report = payload(call("backfill_thumbnails", project=named)) + assert report["examined"] == 1 + assert len(report["filled"]) == 1 + with WorkspaceService.open(root) as service: + project_id = ProjectService(service).get_by_name(named).id + refreshed = IngestService(service).asset(project_id, UUID(report["filled"][0])) + assert refreshed.thumbnail_hash is not None diff --git a/tests/mcp/test_job_tools.py b/tests/mcp/test_job_tools.py new file mode 100644 index 00000000..6ba1fceb --- /dev/null +++ b/tests/mcp/test_job_tools.py @@ -0,0 +1,152 @@ +"""The annotation loop: ``get_job``, the lifecycle, iteration and per-asset progress.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from tests.mcp._flow import BBOX, call, error, ingested, open_batch, payload + + +def _annotate(job_id: str, asset_id: str) -> None: + payload( + call( + "add_annotations", + job_id=job_id, + annotations=[ + { + "asset_id": asset_id, + "label_class": "sign", + "geometry": BBOX, + "provenance": "model", + "model_ref": "probe@1", + } + ], + ) + ) + + +def test_a_job_names_the_batch_and_the_schema_its_work_is_judged_against( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # An `AnnotationJob` records only its task group, so `batch_id` is added here + # — without it a caller holding a job id has no route to the pinned version. + _, batch_id, job_id = open_batch(monkeypatch, tmp_path, count=2) + job = payload(call("get_job", job_id=job_id)) + assert job["batch_id"] == batch_id + assert job["batch_state"] == "in_annotation" + assert job["schema_version"] == 1 + assert job["progress"]["unannotated"] == 2 + + +def test_next_pending_returns_only_unannotated_assets_and_shrinks_as_you_work( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, _, job_id = open_batch(monkeypatch, tmp_path, count=3) + payload(call("start_job", job_id=job_id)) + first = payload(call("next_pending_assets", job_id=job_id, count=10)) + assert first["total"] == 3 + _annotate(job_id, first["items"][0]["id"]) + second = payload(call("next_pending_assets", job_id=job_id, count=10)) + assert second["total"] == 2 + assert first["items"][0]["id"] not in {a["id"] for a in second["items"]} + + +def test_the_loop_terminates_with_an_empty_page( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, _, job_id = open_batch(monkeypatch, tmp_path, count=2) + payload(call("start_job", job_id=job_id)) + for asset in payload(call("next_pending_assets", job_id=job_id, count=10))["items"]: + _annotate(job_id, asset["id"]) + assert payload(call("next_pending_assets", job_id=job_id, count=10)) == { + "items": [], + "total": 0, + } + + +def test_asking_for_no_assets_is_a_malformed_request( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `next_pending` refuses a non-positive count with a bare ValueError, so `ge=1` + # on the parameter has to stop it arriving. + _, _, job_id = open_batch(monkeypatch, tmp_path, count=1) + assert call("next_pending_assets", job_id=job_id, count=0).isError + + +def test_a_job_cannot_be_completed_while_an_asset_is_unsettled( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, _, job_id = open_batch(monkeypatch, tmp_path, count=2) + payload(call("start_job", job_id=job_id)) + assert error(call("complete_job", job_id=job_id))["message"] + + +def test_skipping_an_asset_settles_it_without_writing_anything( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, _, job_id = open_batch(monkeypatch, tmp_path, count=1) + payload(call("start_job", job_id=job_id)) + asset_id = payload(call("next_pending_assets", job_id=job_id, count=1))["items"][0]["id"] + marked = payload( + call("set_asset_progress", job_id=job_id, asset_id=asset_id, progress="skipped") + ) + assert marked == {"asset_id": asset_id, "progress": "skipped"} + assert payload(call("complete_job", job_id=job_id))["state"] == "completed" + + +def test_writing_an_annotation_moves_its_asset_on_its_own( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, _, job_id = open_batch(monkeypatch, tmp_path, count=1) + payload(call("start_job", job_id=job_id)) + asset_id = payload(call("next_pending_assets", job_id=job_id, count=1))["items"][0]["id"] + _annotate(job_id, asset_id) + assert payload(call("get_job", job_id=job_id))["progress"]["annotated"] == 1 + + +def test_re_marking_the_state_an_asset_already_holds_is_not_a_refusal( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, _, job_id = open_batch(monkeypatch, tmp_path, count=1) + asset_id = payload(call("next_pending_assets", job_id=job_id, count=1))["items"][0]["id"] + payload(call("set_asset_progress", job_id=job_id, asset_id=asset_id, progress="skipped")) + payload(call("set_asset_progress", job_id=job_id, asset_id=asset_id, progress="skipped")) + + +def test_an_illegal_progress_move_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `accepted` is terminal, and `review_pending` is only reachable from + # `annotated`. The transition table says so and nothing here restates it. + _, _, job_id = open_batch(monkeypatch, tmp_path, count=1) + asset_id = payload(call("next_pending_assets", job_id=job_id, count=1))["items"][0]["id"] + refusal = error( + call("set_asset_progress", job_id=job_id, asset_id=asset_id, progress="accepted") + ) + assert "cannot become" in refusal["message"] + + +def test_nothing_may_be_written_into_a_batch_nobody_opened( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=1) + approved = payload(call("approve_batch", batch_id=batch_id)) + job_id = approved["jobs"][0]["id"] + asset_id = payload(call("list_batch_assets", batch_id=batch_id))["items"][0]["id"] + assert error(call("start_job", job_id=job_id))["message"] + assert error(call("set_asset_progress", job_id=job_id, asset_id=asset_id, progress="skipped"))[ + "message" + ] + + +def test_an_asset_that_is_not_in_the_job_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + from uuid import uuid4 + + _, _, job_id = open_batch(monkeypatch, tmp_path, count=1) + refusal = error( + call("set_asset_progress", job_id=job_id, asset_id=str(uuid4()), progress="skipped") + ) + assert refusal["message"] diff --git a/tests/mcp/test_project_tools.py b/tests/mcp/test_project_tools.py new file mode 100644 index 00000000..f96265ac --- /dev/null +++ b/tests/mcp/test_project_tools.py @@ -0,0 +1,133 @@ +"""``create_project`` / ``list_projects`` / ``get_project`` / ``delete_project``.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from tests.mcp._flow import call, error, ingested, payload, project, workspace + +from visionset.kernel.services import ProjectService, WorkspaceService + + +def test_creating_a_project_returns_it_with_the_dataset_that_is_its_trunk( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + workspace(monkeypatch, tmp_path) + result = payload(call("create_project", name="road-signs", description="signage")) + assert result["project"]["name"] == "road-signs" + assert result["project"]["description"] == "signage" + # The dataset id is folded in precisely so an agent never has to fetch it. + assert result["dataset"]["project_id"] == result["project"]["id"] + + +def test_a_project_name_collides_case_insensitively( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + project(monkeypatch, tmp_path) + refusal = error(call("create_project", name="ROAD-SIGNS")) + assert "road-signs" in refusal["message"] + assert refusal["retry_with"] is None + + +def test_a_blank_name_is_refused_in_the_envelope( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + workspace(monkeypatch, tmp_path) + assert error(call("create_project", name=" "))["message"] + + +def test_listing_an_empty_workspace_is_a_collection_and_not_an_error( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + workspace(monkeypatch, tmp_path) + assert payload(call("list_projects")) == {"items": [], "total": 0} + + +def test_a_project_is_reachable_by_name_and_by_id( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = project(monkeypatch, tmp_path) + by_name = payload(call("get_project", project=named)) + by_id = payload(call("get_project", project=by_name["project"]["id"])) + assert by_name == by_id + + +def test_a_project_name_differing_only_in_case_still_resolves( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The opposite rule to a release tag, and both live in the kernel beside the + # index that enforces them. + project(monkeypatch, tmp_path) + assert payload(call("get_project", project="ROAD-Signs"))["project"]["name"] == "road-signs" + + +def test_progress_counts_work_and_a_draft_batch_carries_none( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # Progress is tallied over *jobs*, and approval is what creates them — so a + # project whose only batch is still a draft reports zero even though it holds + # three assets. That is the honest answer to "how much work is there", and it + # is why `list_batches` carries `asset_count` separately. + named, _ = ingested(monkeypatch, tmp_path, count=3) + assert payload(call("get_project", project=named))["progress"]["total"] == 0 + + +def test_get_project_reports_progress_once_a_batch_is_approved( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named, batch_id = ingested(monkeypatch, tmp_path, count=3) + payload(call("approve_batch", batch_id=batch_id)) + progress = payload(call("get_project", project=named))["progress"] + assert progress["total"] == 3 + assert progress["unannotated"] == 3 + + +def test_an_unknown_project_is_refused_rather_than_invented( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + workspace(monkeypatch, tmp_path) + assert "nope" in error(call("get_project", project="nope"))["message"] + + +def test_delete_without_confirm_changes_nothing_and_names_the_flag( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + root = tmp_path / "ws" + named = project(monkeypatch, tmp_path) + refusal = error(call("delete_project", project=named)) + assert refusal["retry_with"] == "confirm" + # The refusal is only worth anything if the project is still there afterwards. + with WorkspaceService.open(root) as service: + assert [p.name for p in ProjectService(service).list()] == [named] + + +def test_delete_with_confirm_removes_the_project( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + root = tmp_path / "ws" + named = project(monkeypatch, tmp_path) + assert payload(call("delete_project", project=named, confirm=True))["deleted"]["name"] == named + with WorkspaceService.open(root) as service: + assert ProjectService(service).list() == [] + + +def test_deleting_something_that_is_not_there_says_so_with_or_without_confirm( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + workspace(monkeypatch, tmp_path) + for confirm in (False, True): + assert "nope" in error(call("delete_project", project="nope", confirm=confirm))["message"] + + +def test_no_workspace_configured_is_refused_with_a_remedy_a_client_can_act_on( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The kernel's own sentence ends in "use WorkspaceService.init", a Python call + # an agent cannot make; the hint names what whoever configured the server has + # to do instead. + monkeypatch.setenv("VISIONSET_WORKSPACE", str(tmp_path / "nothing-here")) + monkeypatch.chdir(tmp_path) + refusal = error(call("list_projects")) + assert "VISIONSET_WORKSPACE" in (refusal["hint"] or "") + assert "visionset mcp --workspace" in (refusal["hint"] or "") diff --git a/tests/mcp/test_registration.py b/tests/mcp/test_registration.py new file mode 100644 index 00000000..2cb27b92 --- /dev/null +++ b/tests/mcp/test_registration.py @@ -0,0 +1,176 @@ +"""The tool listing itself: what ships, how it is described, and what it claims to do. + +The listing is the only thing an agent sees before it chooses. These assertions +are about that listing rather than about any tool's behaviour, so they are the +ones that fail when a tool is added carelessly rather than wrongly. +""" + +from __future__ import annotations + +import inspect + +import pytest +from tests.mcp._flow import tool_names, tool_schemas + +from visionset.mcp.main import DESTROYS, TOOLS + +SHIPPED = { + "create_project", + "list_projects", + "get_project", + "delete_project", + "get_schema", + "preview_schema_change", + "create_schema_version", + "ingest", + "list_sources", + "backfill_thumbnails", + "list_batches", + "get_batch", + "approve_batch", + "start_batch", + "complete_batch", + "list_batch_assets", + "promote_batch", + "get_job", + "start_job", + "complete_job", + "next_pending_assets", + "set_asset_progress", + "list_asset_annotations", + "add_annotations", + "update_annotations", + "delete_annotations", + "get_asset_image", + "dataset_stats", + "publish_release", + "list_releases", + "verify_release", + "export_release", + "list_formats", +} +"""Written out rather than derived from ``TOOLS``, so that adding a tool is a +deliberate edit in two places. The ship-vs-fold decision is the whole point of +#35; a set computed from the table would agree with itself no matter what +landed.""" + + +def test_the_server_advertises_exactly_the_shipped_tools() -> None: + assert set(tool_names()) == SHIPPED + + +def test_every_registered_tool_reaches_the_listing() -> None: + # A duplicate name does not raise: FastMCP logs a warning and discards the + # second registration silently, so a copy-paste slip would leave a tool that + # simply is not there. Counting is what catches it. + assert len(tool_names()) == len(TOOLS) + + +def test_thirty_three_tools_ship() -> None: + # The count is a decision, not an accident — 50 candidates were evaluated one + # by one. A change here should be argued in `docs/mcp.md` first. + assert len(SHIPPED) == 33 + + +@pytest.mark.parametrize("name", sorted(SHIPPED)) +def test_every_tool_has_a_description_written_for_an_agent(name: str) -> None: + described = tool_schemas()[name].description + assert described + # `inspect.cleandoc` is applied at registration because FastMCP ships `__doc__` + # raw. Without it every description after the first line arrives indented. + assert not described.startswith(" ") + assert "\n " not in described + + +@pytest.mark.parametrize("name", sorted(SHIPPED)) +def test_every_parameter_of_every_tool_is_documented(name: str) -> None: + # There is no docstring-argument parser anywhere in FastMCP, so a parameter is + # documented only if it carries `Annotated[..., Field(description=...)]`. A + # bare `project: str` tells a model nothing about what to put there. + properties = tool_schemas()[name].inputSchema.get("properties", {}) + undocumented = [p for p, schema in properties.items() if not schema.get("description")] + assert undocumented == [] + + +def test_a_tool_that_can_destroy_data_says_so_and_takes_confirm() -> None: + # `ToolAnnotations` are hints and enforce nothing; `confirm` is what enforces. + # This is what keeps the two from drifting apart, in both directions. + listed = tool_schemas() + gated = { + name for name in SHIPPED if "confirm" in listed[name].inputSchema.get("properties", {}) + } + hinted = { + name + for name in SHIPPED + if listed[name].annotations is not None and listed[name].annotations.destructiveHint + } + assert gated == hinted + + +def test_delete_project_is_the_only_destructive_tool() -> None: + # Deliberate and worth pinning: `delete_annotations` removes rows and is *not* + # destructive in this sense, because the batch gate guards it and deleting a + # label is the ordinary edit loop. + assert {tool.__name__ for tool, hints in TOOLS if hints is DESTROYS} == {"delete_project"} + + +def test_no_tool_administers_tokens() -> None: + # Argued in docs/auth.md with #25: minting a credential is a + # privilege-escalation primitive pointed at the agent's own sandbox, and an + # agent's "shown exactly once" is a transcript. + assert not [name for name in tool_names() if "token" in name] + + +def test_every_tool_name_is_snake_case() -> None: + assert all(name.islower() and " " not in name and "-" not in name for name in tool_names()) + + +def test_the_table_is_in_cycle_order_not_alphabetical() -> None: + # The listing reads as the workflow — make a project, schema, ingest, work, + # promote, publish, export — because that is the order an agent meets them in. + names = tool_names() + assert names.index("create_project") < names.index("create_schema_version") + assert names.index("create_schema_version") < names.index("ingest") + assert names.index("ingest") < names.index("approve_batch") + assert names.index("add_annotations") < names.index("promote_batch") + assert names.index("promote_batch") < names.index("publish_release") + assert names.index("publish_release") < names.index("export_release") + + +def test_every_tool_body_is_wrapped_so_a_refusal_cannot_escape() -> None: + # `guarded` is applied once, in the registration loop. An unwrapped tool would + # ship `str(exc)` to the client prefixed and unstructured, which is the shape + # the error envelope exists to replace. + for tool, _ in TOOLS: + registered = server_tool(tool.__name__) + assert getattr(registered, "__wrapped__", None) is not None + + +def server_tool(name: str) -> object: + """The callable FastMCP actually registered under that name.""" + from visionset.mcp.main import server + + found = server._tool_manager.get_tool(name) + assert found is not None, name + return found.fn + + +def test_the_error_envelope_has_one_shape_everywhere() -> None: + # Four keys, always present, null where they do not apply. A caller that has + # to test for a key's existence before reading it is a caller writing two + # branches for one answer. + from visionset.mcp._errors import refused + + assert set(refused("x")["error"]) == {"message", "retry_with", "hint", "index"} + + +def test_guarded_preserves_the_signature_the_input_schema_is_built_from() -> None: + # `functools.wraps` is load-bearing rather than cosmetic here: without + # `__wrapped__`, `inspect.signature` would report `(*args, **kwargs)` and every + # tool would advertise an empty input schema. + from visionset.mcp import projects + from visionset.mcp._errors import guarded + + assert inspect.signature(guarded(projects.create_project)) == inspect.signature( + projects.create_project + ) diff --git a/tests/mcp/test_release_tools.py b/tests/mcp/test_release_tools.py new file mode 100644 index 00000000..88c26520 --- /dev/null +++ b/tests/mcp/test_release_tools.py @@ -0,0 +1,243 @@ +"""Publishing, verifying and exporting — plus ``dataset_stats`` and ``list_formats``.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest +from tests.mcp._flow import BBOX, SCHEMA_CLASSES, call, error, open_batch, payload + +from visionset.kernel.ports import Exporter + + +def promoted(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, *, count: int = 2) -> str: + """A project whose dataset holds `count` annotated assets. Returns the project.""" + named, batch_id, job_id = open_batch(monkeypatch, tmp_path, count=count) + payload(call("start_job", job_id=job_id)) + for asset in payload(call("next_pending_assets", job_id=job_id, count=count))["items"]: + payload( + call( + "add_annotations", + job_id=job_id, + annotations=[ + { + "asset_id": asset["id"], + "label_class": "sign", + "geometry": BBOX, + "provenance": "model", + "model_ref": "probe@1", + } + ], + ) + ) + payload(call("complete_job", job_id=job_id)) + payload(call("complete_batch", batch_id=batch_id)) + payload(call("promote_batch", batch_id=batch_id)) + return named + + +def test_stats_count_both_annotations_and_the_assets_carrying_them( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=2) + stats = payload(call("dataset_stats", project=named)) + assert stats["asset_count"] == 2 + assert stats["annotated_asset_count"] == 2 + assert stats["annotation_count"] == 2 + # Both totals per class, because they answer different questions. + assert stats["classes"] == [{"label_class": "sign", "annotations": 2, "assets": 2}] + + +def test_a_class_nobody_used_does_not_appear_in_the_stats( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=1) + payload( + call( + "create_schema_version", + project=named, + # The whole contract every time: keeping `sign` exactly as it was is + # what makes this additive. Spelling only the name and geometry would + # drop its `occluded` attribute, which is a narrowing change and needs + # `allow_destructive` — a good demonstration of why `create_schema_version` + # says "a class left out is a class removed". + classes=[*SCHEMA_CLASSES, {"name": "pedestrian", "geometry": "bbox"}], + ) + ) + stats = payload(call("dataset_stats", project=named)) + # What was counted, not what could be: which classes exist is the schema's + # answer and `get_schema` is where it is read. + assert [c["label_class"] for c in stats["classes"]] == ["sign"] + + +def test_publishing_freezes_the_trunk_with_its_counts_and_hash( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=2) + release = payload(call("publish_release", project=named, tag="v1.0")) + assert release["tag"] == "v1.0" + assert release["asset_count"] == 2 + assert release["annotation_count"] == 2 + assert release["schema_version"] == 1 + assert len(release["manifest_hash"]) == 64 + assert release["split"] is None + + +def test_a_split_recipe_is_stored_and_its_fractions_must_sum_to_one( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=2) + recipe: dict[str, Any] = {"train": 0.7, "val": 0.15, "test": 0.15, "seed": 42} + assert ( + payload(call("publish_release", project=named, tag="v1.0", split=recipe))["split"] == recipe + ) + # `SplitRecipe`'s own model validator, reached during argument parsing because + # the domain model is the parameter type. + assert call( + "publish_release", + project=named, + tag="v2.0", + split={"train": 0.5, "val": 0.2, "test": 0.2}, + ).isError + + +def test_publishing_an_empty_dataset_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + from tests.mcp._flow import ingested + + named, _ = ingested(monkeypatch, tmp_path, count=1) + assert error(call("publish_release", project=named, tag="v1.0"))["message"] + + +def test_a_release_tag_is_case_sensitive_unlike_a_project_name( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The two opposite rules, both kernel reads. This is the one that is + # case-*sensitive*, so these are two different releases. + named = promoted(monkeypatch, tmp_path, count=2) + payload(call("publish_release", project=named, tag="v1.0")) + payload(call("publish_release", project=named, tag="V1.0")) + assert {r["tag"] for r in payload(call("list_releases", project=named))["items"]} == { + "v1.0", + "V1.0", + } + assert error(call("publish_release", project=named, tag="v1.0"))["message"] + + +def test_an_unknown_tag_is_refused_rather_than_returning_nothing( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=1) + payload(call("publish_release", project=named, tag="v1.0")) + assert error(call("verify_release", project=named, tag="V1.0"))["message"] + + +def test_verification_of_an_untouched_release_is_ok( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=2) + payload(call("publish_release", project=named, tag="v1.0")) + report = payload(call("verify_release", project=named, tag="v1.0")) + assert report["ok"] is True + assert report["manifest_intact"] is True + assert report["checked"] == 2 + assert report["missing"] == report["corrupt"] == report["cache_mismatches"] == [] + + +def test_only_the_dummy_exporter_is_installed_and_it_is_not_lossy() -> None: + assert payload(call("list_formats")) == { + "items": [{"name": "dummy", "lossy": False}], + "total": 1, + } + + +def test_export_writes_into_the_directory_it_was_given( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=1) + payload(call("publish_release", project=named, tag="v1.0")) + dest = tmp_path / "exports" / "dummy" + result = payload( + call("export_release", project=named, tag="v1.0", format="dummy", dest=str(dest)) + ) + assert result["format"] == "dummy" + assert result["directory"] == str(dest) + assert dest.is_dir() + # `DummyExporter` writes nothing, so zero is an export that ran rather than one + # that failed — and the count comes from walking `dest`, never from the plugin. + assert result["file_count"] == 0 + + +def test_an_unknown_format_names_the_ones_that_are_installed( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=1) + payload(call("publish_release", project=named, tag="v1.0")) + refusal = error( + call("export_release", project=named, tag="v1.0", format="yolo", dest=str(tmp_path / "out")) + ) + assert "dummy" in refusal["message"] + # A `KeyError` here would be outside the VisionSetError tree, so a mistyped + # format has to arrive through `pick`. + assert refusal["retry_with"] is None + + +def test_a_dest_that_is_a_file_is_refused_before_a_plugin_runs( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = promoted(monkeypatch, tmp_path, count=1) + payload(call("publish_release", project=named, tag="v1.0")) + occupied = tmp_path / "not-a-dir" + occupied.write_text("in the way\n") + refusal = error( + call("export_release", project=named, tag="v1.0", format="dummy", dest=str(occupied)) + ) + assert "must be a directory" in refusal["message"] + + +class LossyExporter: + """An exporter that says it cannot carry everything, for the third gate word.""" + + format_name = "lossy-probe" + lossy = True + + def export(self, release: Any, manifest: Any, dest: Path) -> None: + dest.mkdir(parents=True, exist_ok=True) + + +def test_a_lossy_format_refuses_until_its_own_flag_is_passed( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The third gate word, never merged with `confirm` or `allow_destructive`: + # nothing is destroyed and nothing is narrowed, and what is being consented to + # is an incomplete *copy* of something that stays intact. + from visionset.formats import registry + + plugin = LossyExporter() + assert isinstance(plugin, Exporter) + monkeypatch.setattr(registry, "exporters", lambda: {plugin.format_name: plugin}) + + named = promoted(monkeypatch, tmp_path, count=1) + payload(call("publish_release", project=named, tag="v1.0")) + dest = tmp_path / "lossy-out" + refusal = error( + call("export_release", project=named, tag="v1.0", format="lossy-probe", dest=str(dest)) + ) + assert refusal["retry_with"] == "allow_lossy" + assert not dest.exists(), "the gate is checked before anything is created" + + assert ( + payload( + call( + "export_release", + project=named, + tag="v1.0", + format="lossy-probe", + dest=str(dest), + allow_lossy=True, + ) + )["format"] + == "lossy-probe" + ) diff --git a/tests/mcp/test_schema_tools.py b/tests/mcp/test_schema_tools.py new file mode 100644 index 00000000..f415c32a --- /dev/null +++ b/tests/mcp/test_schema_tools.py @@ -0,0 +1,160 @@ +"""``get_schema`` / ``preview_schema_change`` / ``create_schema_version``. + +Also where the two consequences of taking domain models as parameters are pinned: +the domain's own validators refuse malformed input, and a discriminated union's +tag has to be spelled out even though the generated schema shows it as optional. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest +from tests.mcp._flow import SCHEMA_CLASSES, call, error, payload, project, schema, tool_schemas + +CAR_ONLY: list[dict[str, Any]] = [{"name": "car", "geometry": "bbox"}] +BOTH: list[dict[str, Any]] = [*SCHEMA_CLASSES, {"name": "car", "geometry": "bbox"}] + + +def test_a_new_project_has_no_schema_at_all( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # Schema-less on purpose: creating v1 implicitly would be the second door the + # kernel closed. + named = project(monkeypatch, tmp_path) + assert error(call("get_schema", project=named))["message"] + + +def test_the_first_version_is_one_and_it_is_active( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + result = payload(call("get_schema", project=named)) + assert result["schema"]["version"] == 1 + assert result["active_version"] == 1 + assert result["available_versions"] == [1] + + +def test_get_schema_folds_the_version_listing_into_its_answer( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + payload(call("create_schema_version", project=named, classes=BOTH)) + result = payload(call("get_schema", project=named)) + assert result["available_versions"] == [1, 2] + assert result["active_version"] == 2 + assert {c["name"] for c in result["schema"]["classes"]} == {"sign", "car"} + + +def test_an_older_version_can_still_be_read( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + payload(call("create_schema_version", project=named, classes=BOTH)) + older = payload(call("get_schema", project=named, version=1)) + assert [c["name"] for c in older["schema"]["classes"]] == ["sign"] + # Which version was asked for does not change which one is active. + assert older["active_version"] == 2 + + +def test_adding_a_class_is_additive_and_needs_no_flag( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + preview = payload(call("preview_schema_change", project=named, classes=BOTH)) + assert preview["is_destructive"] is False + assert preview["destructive_classes"] == [] + assert payload(call("create_schema_version", project=named, classes=BOTH))["version"] == 2 + + +def test_preview_names_what_a_change_would_remove_without_writing_anything( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + preview = payload(call("preview_schema_change", project=named, classes=CAR_ONLY)) + assert preview["is_destructive"] is True + assert preview["destructive_classes"] == ["sign"] + # Writes nothing: still one version afterwards. That is the whole reason + # `SchemaService.preview` finally has a caller. + assert payload(call("get_schema", project=named))["available_versions"] == [1] + + +def test_a_narrowing_change_is_refused_and_names_the_flag_that_allows_it( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + refusal = error(call("create_schema_version", project=named, classes=CAR_ONLY)) + assert refusal["retry_with"] == "allow_destructive" + assert ( + payload( + call("create_schema_version", project=named, classes=CAR_ONLY, allow_destructive=True) + )["version"] + == 2 + ) + + +def test_a_change_that_would_orphan_annotations_offers_no_flag_at_all( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The distinction a machine-readable `code` was needed for, answered directly: + # `allow_destructive` retries one of these and nothing retries the other. A + # client branching on "it was a 409" would loop here forever. + from tests.mcp._flow import BBOX, open_batch + + named, _, job_id = open_batch(monkeypatch, tmp_path) + asset_id = payload(call("next_pending_assets", job_id=job_id, count=1))["items"][0]["id"] + payload( + call( + "add_annotations", + job_id=job_id, + annotations=[ + { + "asset_id": asset_id, + "label_class": "sign", + "geometry": BBOX, + "provenance": "human", + } + ], + ) + ) + refusal = error( + call("create_schema_version", project=named, classes=CAR_ONLY, allow_destructive=True) + ) + assert refusal["retry_with"] is None + + +def test_a_class_bound_to_an_unimplemented_geometry_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = project(monkeypatch, tmp_path) + refusal = error( + call("create_schema_version", project=named, classes=[{"name": "lane", "geometry": "mask"}]) + ) + assert "mask" in refusal["message"] + + +def test_the_domain_refuses_a_malformed_class_before_the_body_runs( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `classes` is typed `list[LabelClass]`, so the domain's own validators run + # during argument parsing. That is a malformed *request* rather than a domain + # refusal, so it arrives as `isError` with pydantic's own message naming the + # offending field — a deliberately different shape from the error envelope, + # the same split the API makes between 422 and 409. + named = project(monkeypatch, tmp_path) + result = call( + "create_schema_version", project=named, classes=[{"name": " ", "geometry": "bbox"}] + ) + assert result.isError + assert "classes.0.name" in result.content[0].text + + +def test_the_label_class_schema_reaches_the_agent_with_the_domain_docstrings() -> None: + # The reason the domain model goes into the signature at all: FastMCP puts its + # docstring into `$defs`, which is the best guidance an agent gets about what + # a class is. A hand-written body would have thrown it away. + definitions = tool_schemas()["create_schema_version"].inputSchema["$defs"] + assert "LabelClass" in definitions + assert "Attribute" in definitions + assert definitions["LabelClass"]["description"] diff --git a/tests/mcp/test_tool_errors.py b/tests/mcp/test_tool_errors.py new file mode 100644 index 00000000..77cab801 --- /dev/null +++ b/tests/mcp/test_tool_errors.py @@ -0,0 +1,176 @@ +"""The error envelope itself: one shape, and the field that says how to retry. + +There are deliberately **two** failure shapes in this surface, and telling them +apart is the point: + +* a malformed *request* — an argument pydantic refused before the body ran — + arrives as ``isError=True`` carrying pydantic's own message, which names the + offending field. That is the API's 422. +* a *domain refusal* is an ordinary successful call whose payload is the error + envelope. That is the API's 404 or 409, and it is the one a caller branches on. + +Collapsing them would mean either losing the field path on a bad argument or +making every refusal look like a protocol failure. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from tests.mcp._flow import call, error, ingested, payload, project, schema, workspace + +from visionset.kernel import ( + ConfirmationRequired, + DestructiveSchemaChange, + LossyExportNotConsented, + SchemaChangeWouldOrphan, +) +from visionset.mcp._errors import RETRY_WITH, refused + + +def test_the_envelope_always_carries_the_same_four_keys( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # A caller that has to test for a key before reading it is a caller writing + # two branches for one answer. + workspace(monkeypatch, tmp_path) + assert set(error(call("get_project", project="nope"))) == { + "message", + "retry_with", + "hint", + "index", + } + assert set(refused("anything")["error"]) == {"message", "retry_with", "hint", "index"} + + +def test_a_domain_refusal_is_a_result_and_not_a_protocol_error( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + workspace(monkeypatch, tmp_path) + result = call("get_project", project="nope") + assert result.isError is False + assert result.structuredContent is not None + + +def test_a_malformed_argument_is_a_protocol_error_naming_the_field( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = project(monkeypatch, tmp_path) + result = call("get_schema", project=named, version=0) + assert result.isError + assert "version" in result.content[0].text + + +def test_a_missing_required_argument_is_a_protocol_error() -> None: + result = call("get_project") + assert result.isError + + +def test_an_unknown_tool_is_a_protocol_error() -> None: + result = call("no_such_tool") + assert result.isError + + +def test_nothing_leaks_a_traceback(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + # `guarded` catches only `VisionSetError`; every kernel call that raises + # outside the family is guarded at its own call site instead. If one were + # missed, FastMCP would ship the exception's text prefixed with + # "Error executing tool", which is what this looks for. + named = schema(monkeypatch, tmp_path) + for result in ( + call("ingest", project=named, path=str(tmp_path / "nowhere")), + call("ingest", project=named, path=str(tmp_path), fps=-1), + call("get_batch", batch_id="not-a-uuid"), + call("list_batch_assets", batch_id="not-a-uuid"), + ): + assert not result.isError, result.content + assert "Traceback" not in str(result.content) + + +def test_confirm_is_the_retry_word_for_destroying_data( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = project(monkeypatch, tmp_path) + assert error(call("delete_project", project=named))["retry_with"] == "confirm" + + +def test_allow_destructive_is_the_retry_word_for_narrowing_a_contract( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + refusal = error( + call("create_schema_version", project=named, classes=[{"name": "car", "geometry": "bbox"}]) + ) + assert refusal["retry_with"] == "allow_destructive" + + +def test_the_orphan_refusal_offers_nothing_and_that_is_the_whole_point() -> None: + # `SchemaChangeWouldOrphan` is deliberately not a subclass of + # `DestructiveSchemaChange`, so the MRO walk finds nothing — which is what + # stops a client retrying forever. Two refusals that would be the same HTTP + # status, told apart by a field rather than by the status. + assert not issubclass(SchemaChangeWouldOrphan, DestructiveSchemaChange) + assert SchemaChangeWouldOrphan not in RETRY_WITH + + +def test_the_three_gate_words_are_three_and_are_never_merged() -> None: + # `confirm` guards destroying data, `allow_destructive` guards narrowing a + # contract, `allow_lossy` guards emitting an incomplete copy of something that + # stays intact. Different words, different errors, never one `except`. + expected = { + ConfirmationRequired: "confirm", + DestructiveSchemaChange: "allow_destructive", + LossyExportNotConsented: "allow_lossy", + } + assert expected == RETRY_WITH + assert len(set(RETRY_WITH.values())) == 3 + + +def test_most_refusals_are_not_retryable_at_all( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=1) + assert error(call("start_batch", batch_id=batch_id))["retry_with"] is None + assert error(call("get_project", project="nope"))["retry_with"] is None + + +def test_a_bulk_refusal_carries_the_position_and_an_ordinary_one_does_not( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + from tests.mcp._flow import BBOX, open_batch + + _, _, job_id = open_batch(monkeypatch, tmp_path, count=1) + asset_id = payload(call("next_pending_assets", job_id=job_id, count=1))["items"][0]["id"] + bulk = error( + call( + "add_annotations", + job_id=job_id, + annotations=[ + { + "asset_id": asset_id, + "label_class": "nope", + "geometry": BBOX, + "provenance": "human", + } + ], + ) + ) + assert bulk["index"] == 0 + assert error(call("get_job", job_id="not-a-uuid"))["index"] is None + + +def test_the_one_tool_returning_image_content_still_refuses_in_the_envelope( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # `get_asset_image` declares `-> CallToolResult`, and FastMCP would put a + # returned dict into a text block with `structuredContent` null. `guarded` + # wraps the envelope for exactly this tool so a client parses one shape. + from uuid import uuid4 + + named, _ = ingested(monkeypatch, tmp_path, count=1) + result = call("get_asset_image", project=named, asset_id=str(uuid4())) + assert result.structuredContent is not None + assert set(result.structuredContent["error"]) == {"message", "retry_with", "hint", "index"} + # And the text half says the same thing, for a client that reads only content. + assert "error" in result.content[0].text From f297b23a59e8439a4c55686ef0f8de69894ccecf Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Tue, 28 Jul 2026 21:56:49 -0700 Subject: [PATCH 3/3] docs(mcp): docs/mcp.md, and the sibling docs that now point at it The agent surface written down: the thirty-three tools grouped by cycle stage, how a client is configured, the coordinate-frame rule that makes `get_asset_image` safe to annotate from, the error envelope and why it carries `retry_with` instead of a code, the three gate words, the stated limits, and the ship-vs-fold argument for all twenty candidates that did not make it. `cli.md` gets a real `visionset mcp` section in place of the stub note; `workspaces.md` gains the per-call open and why it differs from the server's; `schemas.md` records that `preview` finally has a caller; `auth.md` confirms the no-token-tools decision held when the surface actually shipped. --- README.md | 13 ++- docs/README.md | 1 + docs/auth.md | 5 +- docs/cli.md | 23 +++- docs/mcp.md | 279 +++++++++++++++++++++++++++++++++++++++++++++ docs/schemas.md | 9 +- docs/workspaces.md | 14 +++ 7 files changed, 336 insertions(+), 8 deletions(-) create mode 100644 docs/mcp.md diff --git a/README.md b/README.md index 4b4fa068..9d63e448 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,16 @@ something. `visionset ui` run outside one refuses with one sentence and exit 1; one, because a command that silently made a workspace out of whatever directory you were standing in is how data ends up somewhere nobody chose. +Or hand the workspace to an agent — the same cycle, over +[MCP](https://modelcontextprotocol.io), with the tools an agent needs to *look* at what it is +labelling: + +```json +{ "mcpServers": { "visionset": { + "command": "visionset", "args": ["mcp"], + "env": { "VISIONSET_WORKSPACE": "/path/to/workspace" } } } } +``` + Or drive the whole cycle from the terminal, without a server: ```bash @@ -57,9 +67,10 @@ ffmpeg. ``` src/visionset/ Single Python distribution (one wheel, one import namespace) kernel/ Hexagonal core: domain + ports + default adapters (framework-free) + wire/ The JSON shapes the CLI and MCP publish (gated against the REST models) server/ FastAPI — exposes the SDK via REST; openapi.json is a committed contract cli/ Typer CLI (`visionset` console script) - mcp/ MCP server (stdio) — thin mapping of tools to SDK calls + mcp/ MCP server (stdio) — 33 agent tools over the same SDK formats/ Importer/exporter plugins (entry-point group `visionset.formats`) _static/ Compiled UI bundle lands here at build time (ships in the wheel) frontend/ diff --git a/docs/README.md b/docs/README.md index 975bcc38..0839ee81 100644 --- a/docs/README.md +++ b/docs/README.md @@ -22,4 +22,5 @@ contracts (kernel purity, headless annotator) are described there and enforced i | [examples.md](examples.md) | The two runnable examples: the whole cycle in one pass, ingest on its own, and what each is built to demonstrate | | [api.md](api.md) | The REST surface: the conventions every endpoint follows (paths, UUID ids, the list envelope, gates as query parameters), the one error body, why clients branch on `code` and not on the status, what decides 404 / 409 / 422, what a 5xx does and does not tell you, and which codes are worth retrying | | [auth.md](auth.md) | Who may call it: per-workspace API tokens, why only a digest is stored, why every refusal is one identical 401, immediate revocation, the `visionset token` commands, and how a protected route is built | +| [mcp.md](mcp.md) | The agent surface: the thirty-three tools and what each is for, why fifty candidates became thirty-three, how a client is configured, the coordinate-frame rule that makes `get_asset_image` safe to annotate from, the error envelope and its `retry_with` field, the three gate words, and the stated limits (synchronous ingest and export, local paths, one workspace per server) | | [cli.md](cli.md) | The command line: the whole cycle as a script, the three exit codes (and why one of them also means "no"), why stdout is data and stderr is prose, what `--json` promises and how it stays the API's shape, why `--workspace` follows the subcommand, and what `visionset init` and `visionset ui` each do | diff --git a/docs/auth.md b/docs/auth.md index 855eb349..31766afd 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -181,4 +181,7 @@ that outlives the session. The secret is shown exactly once, and an agent's "onc transcript: `confirm: true` guards accidental mutation, not exfiltration. Whoever launched `visionset mcp` already had workspace access, so a second credential adds capability and subtracts accountability. `list_tokens` is the only defensible candidate and is still operator surface -rather than dataset surface. +rather than dataset surface. That held when #35 shipped the surface: none of its thirty-three +tools touches a token, and none needs one — an agent reaching the MCP server is already inside the +sandbox the workspace defines, so there is nothing further to prove. Authentication is what an +*HTTP* client owes, because the network is what a token is for. diff --git a/docs/cli.md b/docs/cli.md index 2586b5dc..e91d1308 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -37,7 +37,7 @@ visionset token create --name NAME visionset token list visionset token revoke NAME [--yes] visionset ui [--host] [--port] [--reload] # no --json -visionset mcp # not implemented yet +visionset mcp # stdio; no --json ``` ## The cycle, as a script @@ -358,9 +358,24 @@ digest is stored, and why revocation does not free the name — in [auth.md](aut ## `visionset mcp` -A stub. The MCP server is a fourth sibling client of the same SDK; the command that starts it names -its target by import string or subprocess for the same reason `ui` does — import-linter forbids -`visionset.cli` importing `visionset.mcp`. +Starts the MCP server on stdio, serving this workspace to an agent. Thirty-three tools covering +the whole cycle; [mcp.md](mcp.md) has the list, how to configure a client, and what a tool refusal +looks like. + +``` +visionset mcp [--workspace PATH] +``` + +Normally a client spawns it rather than a person running it. Like `ui`, it resolves the workspace +with the full precedence and then **states** the answer in `VISIONSET_WORKSPACE`, so the server it +starts cannot disagree with it, and it opens the workspace first so that `NotAWorkspace` is one +sentence at exit 1 rather than a refusal inside the agent's first tool call. + +The target is named as a module for a subprocess rather than imported, for the reason `ui` names +uvicorn's app by import string — import-linter forbids `visionset.cli` importing `visionset.mcp`. +The subprocess inherits stdin and stdout, because those two streams *are* the transport, which is +also why this is the one command that prints **nothing at all** on stdout: a stray line would +corrupt the JSON-RPC stream before the first message. ## For contributors diff --git a/docs/mcp.md b/docs/mcp.md new file mode 100644 index 00000000..6d82807f --- /dev/null +++ b/docs/mcp.md @@ -0,0 +1,279 @@ +# The MCP server + +VisionSet speaks [Model Context Protocol](https://modelcontextprotocol.io) over stdio, so an +agent can run the whole cycle — create a project, declare a schema, ingest images, **look at +them**, write annotations, promote, publish, verify and export — without a browser, an HTTP +client or a line of Python. + +It is the fourth client of the same kernel the API, the CLI and the SDK use. Nothing is decided +here that is not decided in the kernel, and every rule the other surfaces follow holds unchanged. + +```bash +visionset mcp --workspace ~/datasets/road-signs +``` + +## Configuring a client + +A client spawns the server itself and talks to it on stdin and stdout. The workspace travels in +the environment, because there is no other channel: + +```json +{ + "mcpServers": { + "visionset": { + "command": "visionset", + "args": ["mcp"], + "env": { "VISIONSET_WORKSPACE": "/home/you/datasets/road-signs" } + } + } +} +``` + +`python -m visionset.mcp.main` is the same server without the pre-flight check, for a client that +cannot run a console script. + +`visionset mcp` resolves the workspace with the full precedence documented in +[workspaces.md](workspaces.md) — `--workspace`, then `$VISIONSET_WORKSPACE`, then the nearest +workspace at or above the working directory — and then **states** the answer in the environment, +so the server it starts cannot disagree with it. It also opens the workspace before starting +anything, which runs any pending migration and turns "that is not a workspace" into one sentence +at a terminal instead of a refusal inside the agent's first tool call. + +**No token is involved.** A token authenticates an HTTP client; an agent reaching this server is +already inside the sandbox the workspace defines, and there is nothing further to prove. There +are no token-administration tools either, for the reason [auth.md](auth.md) gives: minting a +credential is a privilege-escalation primitive pointed at the agent's own sandbox, and an agent's +"shown exactly once" is a transcript. + +## The tools + +Thirty-three, listed in the order an agent meets them. + +### Projects and schema + +| | | +| --- | --- | +| `create_project` | Make a project and the empty dataset that is its trunk. | +| `list_projects` | Everything in this workspace. | +| `get_project` | The project, its dataset id, and how far its work has got. | +| `delete_project` | **Destructive.** Requires `confirm: true`. | +| `get_schema` | The classes, the active version, and every version that exists. | +| `preview_schema_change` | What a proposed change would do. Writes nothing. | +| `create_schema_version` | Apply one. `allow_destructive` for a narrowing change. | + +### Sources and ingest + +| | | +| --- | --- | +| `ingest` | A local path in, one batch out. Synchronous. | +| `list_sources` | The folders and clips a project was built from. | +| `backfill_thumbnails` | Render previews that are missing. | + +### Batches + +| | | +| --- | --- | +| `list_batches` | Outstanding work, with progress. | +| `get_batch` | One batch: state, schema pin, progress, jobs. | +| `approve_batch` | Freeze it, pin the schema, cut it into jobs. | +| `start_batch` | Open it for annotation. | +| `list_batch_assets` | What is in it, paged, with each asset's job and progress. | +| `complete_batch` | Close it, once every job is complete. | +| `promote_batch` | Move the finished assets into the dataset. | + +### Jobs and annotations + +| | | +| --- | --- | +| `get_job` | State, counts, and the batch and schema it answers to. | +| `start_job` | Mark it as being worked on. | +| `next_pending_assets` | The loop primitive: what is left to annotate. | +| `get_asset_image` | **Look at the pixels.** See below. | +| `list_asset_annotations` | What is already on an asset, with ids for editing. | +| `add_annotations` | Write labels. All or none. | +| `update_annotations` | Replace labels wholesale, by id. All or none. | +| `delete_annotations` | Remove labels. All or none. No confirmation. | +| `set_asset_progress` | Say an asset is `skipped`, or move it otherwise. | +| `complete_job` | Close it, once every asset is settled. | + +### Datasets, releases and export + +| | | +| --- | --- | +| `dataset_stats` | Class balance in the trunk right now. | +| `publish_release` | Freeze it under a tag, immutably. | +| `list_releases` | Everything published, with counts and hashes. | +| `verify_release` | Re-hash every blob a release names. | +| `list_formats` | Installed exporters, and which are lossy. | +| `export_release` | Write a release to a local directory. `allow_lossy` where needed. | + +## `get_asset_image`, and the coordinate frame + +This is the tool that makes an agent an annotator rather than an operator. Everything else moves +rows around; without this one a model can drive the whole workflow and never see what it is +labelling, and `add_annotations` with `provenance: "model"` means nothing unless the model looked. + +It returns the image **and four numbers**, because they are not the same frame: + +```json +{ "asset_id": "…", "width": 4032, "height": 3024, "format": "jpeg", + "image_width": 256, "image_height": 192, "resolution": "thumbnail", "scale": 15.75 } +``` + +- `width` / `height` are the **asset's own size**, and that is the coordinate system every + annotation uses. Geometry is never normalized, at any surface. +- `image_width` / `image_height` are what was actually sent. The default is the cached preview, + capped at 256 on its long edge, because the bytes travel base64-encoded inside a single + JSON-RPC message and an original would cost an agent its context window. +- `scale` is the factor between them. **Multiply any coordinate measured on the returned image + by `scale` before writing it.** + +That last line is the whole reason four numbers travel instead of two. An agent that measures a +box on a 256-pixel preview and submits it unscaled produces annotations that are individually +plausible and uniformly wrong — wrong in a way nothing downstream can detect, because every +number is in range and every shape is well formed. + +`full: true` returns the original bytes at the asset's own size, where `scale` is 1. + +An asset whose preview has never been rendered is refused, and the refusal names +`backfill_thumbnails`. An asset with no recorded image format has no honest media type, so its +measurements come back with an explanation rather than pixels. + +## How a tool refuses + +There are **two** failure shapes, and telling them apart is deliberate. + +A **malformed request** — an argument the schema refuses before the tool body runs — comes back +as an MCP error carrying the validating library's own message, which names the offending field +(`classes.0.name`, `annotations.2.geometry`). This is the API's 422, and it is a bug in the call. + +A **domain refusal** is an ordinary successful call whose payload is the error envelope: + +```json +{ "error": { "message": "…", "retry_with": "allow_destructive", "hint": null, "index": null } } +``` + +- `message` is the kernel's own sentence. It is written to be read, and an agent reads. +- `retry_with` names the parameter that turns this exact call into a successful one, or is + `null` when nothing does. **This is what to branch on.** `DESTRUCTIVE_SCHEMA_CHANGE` is + retryable with `allow_destructive` and `SCHEMA_CHANGE_WOULD_ORPHAN` is not; over HTTP those are + both 409, and a client branching on the status would retry the second one forever. +- `hint` is a next step this surface can suggest where the kernel's sentence names a remedy an + agent cannot reach. +- `index` names which item of a list you sent is at fault, for the three annotation writes. They + are all-or-nothing, so nothing landed and there is no partial result to count from — the + position is the only thing identifying it. + +There is deliberately **no `code` field**. The REST API's codes live in `server/errors.py`, which +this package may not import, and deriving one from a class name would make a refactor a silent +breaking change. What a code was needed for here is one question — "may I retry this, and with +what?" — and `retry_with` answers it directly. + +### The three gate words + +Never merged into one, because they guard different things: + +| | guards | on | +| --- | --- | --- | +| `confirm` | destroying data | `delete_project` | +| `allow_destructive` | narrowing a contract | `create_schema_version` | +| `allow_lossy` | emitting an incomplete copy of something that stays intact | `export_release` | + +`delete_annotations` takes **none** of them. Removing a label is the annotator edit loop, and the +guard is that a batch which is no longer `in_annotation` refuses every write. + +## Stated limits + +**Ingest and export are synchronous.** A stdio server has no background worker: something has to +do the decode, and an agent driving a "resume" loop would block for exactly as long as doing the +work in the first place. A long video makes `ingest` a long call. + +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 +gave the CLI no `--resume`. + +**Paths are local.** `ingest` and `export_release` take paths on the machine the server runs on. +The API's upload staging exists because HTTP has bytes where the kernel has paths; an agent runs +beside the workspace and has the filesystem. + +**One workspace per server.** No tool takes a workspace parameter — threading one through +thirty-three 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 ui` or +a second agent between calls. + +**A discriminated union's `type` must be spelled out.** `geometry` and the partition variants +carry a default on their tag, so the generated schema shows `type` as optional — but it is read +out of the object to pick the variant, and omitting it fails. Always send +`{"type": "bbox", …}`. + +## What is not here, and why + +Fifty candidate tools were recorded across the four REST tasks; thirty ship, twenty do not. 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. + +**Folded into a parent**, because the parent already reads it and a second tool is a second round +trip: `get_project_dataset`, `get_dataset`, `list_schema_versions`, `get_source`, +`list_batch_jobs`, `get_job_progress`, `get_asset`, `get_release`. + +**Folded into `ingest`**: `register_image_source`, `register_video_source`, `start_ingest`. The +kernel splits registration in two because a clip needs a rate and a probe while a folder needs +neither; by the time ingest runs, the source already carries the kind, the path and the rate. The +dispatch is whether the path is a directory. + +**Dropped, no poll to make**: `get_ingest_job`, `list_ingest_jobs`, `resume_ingest` — see the +synchronous limit above. + +**Dropped, no agent caller**: `list_dataset_assets` (the annotation loop iterates batches, not the +trunk), `list_dataset_changes` (an audit record a person reads), `remove_dataset_asset` +(curation — a judgement about what a dataset should contain, not a step in producing one), +`get_release_manifest` (the whole frozen document is a token bill an agent cannot afford; +`verify_release` answers "is it intact" and `export_release` writes the contents somewhere +usable), `get_release_assignment` (`export_release` puts the folds on disk in the form anything +downstream actually consumes), `rename_project`. + +**Never offered**: anything to do with tokens. + +Three tools are not on the parity list at all: `ingest` (one tool standing for three candidates), +`preview_schema_change` and `backfill_thumbnails` — the last because it is the remedy +`get_asset_image` names, and a refusal naming an unreachable remedy is worse than no refusal. + +## For contributors + +Tool modules live in `src/visionset/mcp/`, one per noun, beside three private ones: `_errors.py` +(the envelope and `guarded`), `_workspace.py` (`opened_workspace()`) and `_resolve.py` (turning a +name or a tag into the thing it names). + +A new tool is a plain function in the module for its noun plus one row in `main.py`'s `TOOLS` +table. Registration lives there rather than at the definition site — a decorator in `projects.py` +would make that module import `main.py`, which imports it — and it is also where `guarded`, +`inspect.cleandoc` and the read/write annotations are applied, so none of the three can be +forgotten. + +**Take domain models as parameters.** `list[LabelClass]`, `Geometry`, `SplitRecipe` and +`AssetProgress` all go straight into signatures: their docstrings become `$defs` on the tool's +input schema, which is the best guidance an agent gets, and their own validators refuse malformed +input without anything being restated. The exception is a model with a field the service +overwrites — `Annotation.schema_version` — where a required input whose value is discarded would +be a lie, so `mcp/annotations.py` defines the two input models that omit it. + +**Publish through `visionset.wire`**, never `model_dump()`. The projections there are shared with +the CLI and gated key-for-key against the REST wire models by +`tests/cli/test_json_contract.py`, so one concept has one shape across all three surfaces. + +**Mirror every domain bound in the parameter.** A kernel call that raises outside the +`VisionSetError` tree — a bare `ValueError` from a non-positive rate, a `FileNotFoundError` from a +missing path, a pydantic `ValidationError` from constructing a `BySize` — never reaches `guarded`, +and would arrive at the client as an exception's text. Either bound the parameter (`ge=1`) or +refuse in the body with `_errors.refused`. + +Tests are in `tests/mcp/`, and every one drives the **real protocol** through +`create_connected_server_and_client_session` rather than calling the Python function. `_flow.py` +bridges the async client with `anyio.run`, so every test is plain synchronous pytest with no +marker and no plugin, and it builds each rung by calling tools rather than by reaching past them +into the SDK. + +Module basenames must be unique across the whole suite — there is no `__init__.py` anywhere, so +`tests/mcp/test_batches.py` beside `tests/server/test_batches.py` would be a collection error +rather than two modules. Hence `test__tools.py`. diff --git a/docs/schemas.md b/docs/schemas.md index c50a8b9c..800795be 100644 --- a/docs/schemas.md +++ b/docs/schemas.md @@ -38,8 +38,13 @@ with WorkspaceService.open("./road-signs") as workspace: **Over HTTP:** `POST`/`GET /projects/{project_id}/schema/versions`, `GET /projects/{project_id}/schema/versions/{version}`, and `GET /projects/{project_id}/schema` for the version in force. Narrowing needs `?allow_destructive=true`, exactly as -`allow_destructive=` does here. `preview`, `compare` and `allowed_geometries` have no route yet — -they will get one when a surface needs them. See [api.md](api.md). +`allow_destructive=` does here. `compare` and `allowed_geometries` have no route yet — they will +get one when a surface needs them. See [api.md](api.md). + +**Over MCP:** `get_schema`, `create_schema_version` and `preview_schema_change`. The last is +`preview`'s first caller anywhere: plan-before-apply matters most for the surface that cannot see +the consequences of a change until it has made one, so an agent gets the diff before it decides +whether it needs `allow_destructive`. See [mcp.md](mcp.md). ## Versions are 1..N, and none of them changes diff --git a/docs/workspaces.md b/docs/workspaces.md index 42eec722..4355ec1d 100644 --- a/docs/workspaces.md +++ b/docs/workspaces.md @@ -183,6 +183,20 @@ and then *states* it in `VISIONSET_WORKSPACE` — one decision, made once, at th standing at. It has to travel that way rather than as an argument, because `create_app()` takes no parameters and `--reload` runs the application in a separate process. +**The MCP server is the same story with a different reason.** `visionset mcp` resolves through all +four branches and states the answer, so the child it spawns reaches only 2; the child takes no +arguments because stdin and stdout are the transport and there is nowhere else to put one. An MCP +client that spawns `visionset` directly sets `VISIONSET_WORKSPACE` in the server entry's own `env` +— see [mcp.md](mcp.md). + +**How long a workspace stays open differs by surface, deliberately.** The HTTP server builds one +handle in `create_app()` and keeps it for the process's life, because it is one long-lived reader +of many requests. The CLI and the MCP server open and close per command and per *tool call*: there +is then no module-level state to tear down between tests, and — since SQLite has one writer — a +stdio server that held the file between calls would keep `visionset ui` and a second agent out of +a workspace nobody is using. `close()` checkpoints the WAL, so neither leaves a `visionset.db-wal` +behind. + **Only case 3 walks, and that asymmetry is the whole rule.** A flag and an environment variable are somebody *stating* which workspace. If the stated directory holds none, walking to its parent and quietly minting a credential into whatever workspace lives up there is the worst thing this