From 94dfab0a6a7032f060a45451ab23ae93a3fe1e7e Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:31:55 -0700 Subject: [PATCH 1/4] feat(cli): approve --start, complete --promote and ingest --start Each flag is the next lifecycle command made in the same run: approve then start, complete then promote, and for ingest approve as one job then start. No transition is added; start needs only approved and promote only completed, so the second call is legal whenever the first succeeded. Each step commits on its own, and a refused second step prints the first step's line, the refusal as the single-step command prints it, and a line naming the step that refused and the state the batch is in. Plain output reports both outcomes and keeps the batch id alone on stdout. With --json, approve --start and ingest --start print the started batch; complete --promote prints {"batch": ..., "promoted": ...}, the closed batch beside the page of assets that entered the dataset. --- src/visionset/cli/batches.py | 126 +++++++++++++++++++++++------ src/visionset/cli/ingest.py | 50 +++++++++++- tests/cli/test_batch_commands.py | 129 ++++++++++++++++++++++++++++++ tests/cli/test_ingest_commands.py | 46 +++++++++++ 4 files changed, 325 insertions(+), 26 deletions(-) diff --git a/src/visionset/cli/batches.py b/src/visionset/cli/batches.py index 45f7df8c..f757abbd 100644 --- a/src/visionset/cli/batches.py +++ b/src/visionset/cli/batches.py @@ -5,6 +5,14 @@ ``complete``, then ``promote``; ``pre-label`` invokes shared inference inline, because a terminal has no dispatcher. +**A composed flag is two of those calls behind one command, never a new +transition.** ``approve --start`` and ``complete --promote`` make the first +call, and on its success the second; ``ingest --start`` reaches in here for the +same two. Each step commits on its own, so a refused second step leaves the +first one's state in place and the output names it — the kernel's own +``start`` requires only ``approved``, and ``promote`` only ``completed``, which +is what makes the pair safe to chain without any new rule. + **There is no ``batch create``, and none of the membership commands.** 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. ``BatchService`` still has @@ -26,8 +34,10 @@ from __future__ import annotations +from collections.abc import Iterator +from contextlib import contextmanager from dataclasses import asdict -from typing import Annotated, Final +from typing import Annotated, Any, Final from uuid import UUID import typer @@ -46,9 +56,12 @@ pre_label, shapes_prose, ) +from visionset.kernel import VisionSetError from visionset.kernel.domain import ( SETTLED_PROGRESS, + Asset, AssetProgress, + Batch, BySize, GeometryType, Partition, @@ -94,7 +107,7 @@ def _echo(batch_id: UUID, state: str, json_out: bool, payload: dict[str, object]) -> None: - """One shape for the four lifecycle commands: the batch, or a line and its id.""" + """One shape for a lifecycle move: the batch, or a line and its id.""" if json_out: document(payload) return @@ -113,6 +126,44 @@ def _promoted(service: WorkspaceService, project_id: UUID) -> frozenset[UUID]: return DatasetService(service).member_asset_ids(dataset.id) +def batch_document(service: WorkspaceService, batch: Batch) -> dict[str, Any]: + """One batch as ``--json`` prints it, with its progress and its trunk membership read.""" + counts = JobService(service).batch_progress(batch.id) + pre_labeled = BatchService(service).latest_pre_label_job(batch.id) + return wire.batch( + batch, counts, promoted=_promoted(service, batch.project_id), pre_labeled=pre_labeled + ) + + +def approved_note(service: WorkspaceService, approved: Batch) -> None: + """The line approval prints: the pin, and how many jobs it cut.""" + job_count = len(BatchService(service).jobs(approved.id)) + note( + f"Approved batch {approved.name!r} against schema version " + f"{approved.schema_version}, in {job_count} job(s)." + ) + + +@contextmanager +def second_step(step: str, batch_id: UUID, state: str) -> Iterator[None]: + """A later step of a composed command: a refusal says where the batch stands. + + The earlier step has already committed, so the refusal's sentence alone would + leave a reader guessing whether anything moved. + """ + try: + yield + except VisionSetError: + note(f"The {step} step refused; batch {batch_id} is {state}.") + raise + + +def start_after_approval(service: WorkspaceService, approved: Batch) -> Batch: + """The ``start`` half of a composed approval, for ``approve --start`` and ``ingest --start``.""" + with second_step("start", approved.id, approved.state.value): + return BatchService(service).start(approved.id) + + @batch_app.command("list") def batch_list( project: ProjectOption, @@ -173,6 +224,13 @@ def batch_approve( help="Cut into jobs of this many assets. Default: one job for the whole batch.", ), ] = None, + start: Annotated[ + bool, + typer.Option( + "--start", + help="Also open the batch for annotation once it is approved, as `batch start` would.", + ), + ] = False, json_out: JsonOption = False, workspace: WorkspaceOption = None, ) -> None: @@ -181,25 +239,27 @@ def batch_approve( Approval is one-way. There is no route back to `draft`, because the jobs are already partitioned against the pinned schema version — and a later `schema apply` does not move that pin. + + `--start` follows with `batch start`. Approval is committed before the start + is attempted, so a start that is refused leaves an approved batch, and the + output says so. """ # ``min=1`` rather than a check in the body: ``BySize.size`` is ``gt=0``, and # a pydantic ``ValidationError`` from constructing one is not a # ``VisionSetError``, so Click has to refuse zero before the domain sees it. partition: Partition | None = None if jobs_of is None else BySize(size=jobs_of) with opened_workspace(workspace) as service: - batches = BatchService(service) - approved = batches.approve(batch, partition) - counts = JobService(service).batch_progress(approved.id) - job_count = len(batches.jobs(approved.id)) - promoted = _promoted(service, approved.project_id) + approved = BatchService(service).approve(batch, partition) + if not json_out: + approved_note(service, approved) + final = start_after_approval(service, approved) if start else approved + payload = batch_document(service, final) if json_out: - document(wire.batch(approved, counts, promoted=promoted)) + document(payload) return - note( - f"Approved batch {approved.name!r} against schema version " - f"{approved.schema_version}, in {job_count} job(s)." - ) - typer.echo(str(approved.id)) + if start: + note(f"Batch {final.id} is now {final.state.value}.") + typer.echo(str(final.id)) @batch_app.command("start") @@ -327,6 +387,14 @@ def batch_pre_label( @batch_app.command("complete") def batch_complete( batch: BatchArgument, + promote: Annotated[ + bool, + typer.Option( + "--promote", + help="Also move the finished assets into the dataset once the batch is closed, as " + "`batch promote` would.", + ), + ] = False, json_out: JsonOption = False, workspace: WorkspaceOption = None, ) -> None: @@ -334,19 +402,29 @@ def batch_complete( Derived means recomputed, not automatic: this reads the jobs and refuses while any is outstanding. + + `--promote` follows with `batch promote`. With `--json` the document is then + `{"batch": …, "promoted": …}` — the closed batch beside the page of assets + that entered the dataset — and stdout otherwise stays the batch id alone. """ with opened_workspace(workspace) as service: - batches = BatchService(service) - completed = batches.complete(batch) - counts = JobService(service).batch_progress(completed.id) - promoted = _promoted(service, completed.project_id) - pre_labeled = batches.latest_pre_label_job(completed.id) - _echo( - completed.id, - completed.state.value, - json_out, - wire.batch(completed, counts, promoted=promoted, pre_labeled=pre_labeled), - ) + completed = BatchService(service).complete(batch) + if not json_out: + note(f"Batch {completed.id} is now {completed.state.value}.") + entered: list[Asset] = [] + if promote: + with second_step("promote", completed.id, completed.state.value): + entered = DatasetService(service).promote(completed.id, actor=_ACTOR) + payload = batch_document(service, completed) + if json_out: + if promote: + document({"batch": payload, "promoted": wire.page([wire.asset(a) for a in entered])}) + else: + document(payload) + return + if promote: + note(f"Promoted {len(entered)} asset(s) into the dataset.") + typer.echo(str(completed.id)) @batch_app.command("promote") diff --git a/src/visionset/cli/ingest.py b/src/visionset/cli/ingest.py index ff91681b..401a7d24 100644 --- a/src/visionset/cli/ingest.py +++ b/src/visionset/cli/ingest.py @@ -27,6 +27,12 @@ BATCH=$(visionset ingest ./incoming --project road-signs) +``--start`` keeps that rule and takes the batch through ``approve`` (one job) +and ``start`` in the same run, borrowing ``cli/batches.py``'s two halves so the +lines it prints are the ones those commands print. The ingest has committed +before approval is attempted; a project with no schema refuses there and leaves +the draft the ingest made, which the output names. + ``backfill-thumbnails`` lives here rather than under a group because it has no object group to join and it is the other half of what ingest writes: a preview is a cache, so a missing one is a thing to fill in later rather than a failure to @@ -45,9 +51,21 @@ 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 -from visionset.kernel.domain import IngestFailure, IngestFailureKind, IngestResult, TimeRange +from visionset.cli.batches import ( + approved_note, + batch_document, + second_step, + start_after_approval, +) +from visionset.kernel.domain import ( + BatchState, + IngestFailure, + IngestFailureKind, + IngestResult, + TimeRange, +) from visionset.kernel.ports import DEFAULT_EXTRACTION_FPS -from visionset.kernel.services import IngestService, SourceService +from visionset.kernel.services import BatchService, IngestService, SourceService _FAILURE_COLUMNS: Final = ("FILE", "KIND", "REASON") @@ -145,6 +163,14 @@ def ingest( help="Name the batch this run fills. Defaults to the source's own name.", ), ] = None, + start: Annotated[ + bool, + typer.Option( + "--start", + help="Also approve the batch as one job and open it for annotation, as " + "`batch approve --start` would. With --json, prints the started batch.", + ), + ] = False, json_out: JsonOption = False, workspace: WorkspaceOption = None, ) -> None: @@ -156,6 +182,10 @@ def ingest( Files are addressed by content, so ingesting the same bytes twice gives one asset. That is what makes re-running this safe after an interruption. + + `--start` follows with `batch approve` (one job) and `batch start`. The + ingest is committed before approval is attempted, so a refused approval — a + project with no schema — leaves a draft batch, and the output names it. """ # ``typer.Option`` can express ``min=`` but not Click's ``min_open``, so a # ``gt=0`` bound has to be checked here. It has to be checked *somewhere*: @@ -188,7 +218,23 @@ def ingest( ) note(f"Reading {registered.kind.value.replace('_', ' ')} {source}…") result = IngestService(service).ingest(registered.id, batch_name=batch_name) + if start: + if not json_out: + _report(result) + with second_step("approve", result.batch_id, BatchState.DRAFT.value): + approved = BatchService(service).approve(result.batch_id, None) + if not json_out: + approved_note(service, approved) + started = start_after_approval(service, approved) + started_document = batch_document(service, started) + if start: + if json_out: + document(started_document) + return + note(f"Batch {started.id} is now {started.state.value}.") + typer.echo(str(started.id)) + return if json_out: document( { diff --git a/tests/cli/test_batch_commands.py b/tests/cli/test_batch_commands.py index 50684e12..e6baa7a5 100644 --- a/tests/cli/test_batch_commands.py +++ b/tests/cli/test_batch_commands.py @@ -303,6 +303,135 @@ def test_complete_closes_a_finished_batch(root: Path, tmp_path: Path) -> None: assert payload(root, "batch", "list", "-p", name)["items"][0]["state"] == "completed" +# --- approve --start --------------------------------------------------------- + + +def _state(root: Path, name: str, batch: str) -> str: + listed = {row["id"]: row for row in payload(root, "batch", "list", "-p", name)["items"]} + return str(listed[batch]["state"]) + + +def test_approve_start_opens_the_batch_and_reports_both_steps(root: Path, tmp_path: Path) -> None: + name, batch = ingested_batch(root, tmp_path) + + result = run(root, "batch", "approve", batch, "--start") + + assert result.exit_code == 0, result.output + assert result.stdout == f"{batch}\n" + assert "Approved batch 'stills' against schema version 1, in 1 job(s)." in result.stderr + assert f"Batch {batch} is now in_annotation." in result.stderr + assert _state(root, name, batch) == "in_annotation" + + +def test_approve_start_json_prints_the_started_batch(root: Path, tmp_path: Path) -> None: + _, batch = ingested_batch(root, tmp_path) + document = payload(root, "batch", "approve", batch, "--jobs-of", "3", "--start") + assert document["id"] == batch + assert document["state"] == "in_annotation" + assert document["schema_version"] == 1 + assert len(jobs_of(root, batch)) == 2 + + +def test_approve_start_without_a_schema_refuses_and_leaves_a_draft( + root: Path, tmp_path: Path +) -> None: + """The refusal is approve's own: nothing has moved, and the output says so.""" + project(root, "bare") + batch = ok(root, "ingest", str(stills(tmp_path)), "--project", "bare") + + result = run(root, "batch", "approve", batch, "--start") + + assert result.exit_code == 1, result.output + assert result.stdout == "" + assert "Error:" in result.stderr + assert "in_annotation" not in result.stderr + assert _state(root, "bare", batch) == "draft" + + +# --- complete --promote ------------------------------------------------------ + + +def _finish_jobs(root: Path, batch: str) -> None: + """Every asset of every job marked ``annotated`` and every job closed.""" + for job in jobs_of(root, batch): + ok(root, "job", "start", job) + for line in ok(root, "job", "next", job, "-n", "100").splitlines()[1:]: + ok(root, "job", "mark", job, line.split()[0], "--progress", "annotated") + ok(root, "job", "complete", job) + + +def test_complete_promote_closes_the_batch_and_fills_the_trunk(root: Path, tmp_path: Path) -> None: + name, batch = started_batch(root, tmp_path) + _finish_jobs(root, batch) + + result = run(root, "batch", "complete", batch, "--promote") + + assert result.exit_code == 0, result.output + assert result.stdout == f"{batch}\n" + assert f"Batch {batch} is now completed." in result.stderr + assert "Promoted 6 asset(s) into the dataset." in result.stderr + assert _trunk_size(root, name) == 6 + + +def test_complete_promote_json_carries_the_batch_and_the_promoted_page( + root: Path, tmp_path: Path +) -> None: + _, batch = started_batch(root, tmp_path) + _finish_jobs(root, batch) + + document = payload(root, "batch", "complete", batch, "--promote") + + assert document["batch"]["id"] == batch + assert document["batch"]["state"] == "completed" + assert document["batch"]["promoted_asset_count"] == 6 + assert document["promoted"]["total"] == 6 + assert len(document["promoted"]["items"]) == 6 + + +def test_complete_promote_over_assets_already_in_the_trunk_promotes_nothing( + root: Path, tmp_path: Path +) -> None: + """Promotion has no refusal of its own once `complete` has succeeded — `start` + needs only `approved` and `promote` only `completed` — so the outcome left to + pin is the idempotent one: a second batch over assets the trunk already + holds closes, and the promote step reports zero rather than failing.""" + name, first = completed_batch(root, tmp_path) + ok(root, "batch", "promote", first) + second = ok( + root, "ingest", str(tmp_path / "incoming"), "--project", name, "--batch-name", "again" + ) + ok(root, "batch", "approve", second, "--start") + _finish_jobs(root, second) + + result = run(root, "batch", "complete", second, "--promote") + + assert result.exit_code == 0, result.output + assert f"Batch {second} is now completed." in result.stderr + assert "Promoted 0 asset(s) into the dataset." in result.stderr + assert payload(root, "batch", "promote", second) == {"items": [], "total": 0} + assert _trunk_size(root, name) == 6 + + +def test_complete_promote_with_an_outstanding_job_refuses_and_promotes_nothing( + root: Path, tmp_path: Path +) -> None: + name, batch = started_batch(root, tmp_path) + + result = run(root, "batch", "complete", batch, "--promote") + + assert result.exit_code == 1, result.output + assert result.stdout == "" + assert "Error:" in result.stderr + assert _trunk_size(root, name) == 0 + + +def test_batch_help_lists_the_composed_flags() -> None: + approve = runner.invoke(app, ["batch", "approve", "--help"], env=RENDERING, color=True) + complete = runner.invoke(app, ["batch", "complete", "--help"], env=RENDERING, color=True) + assert "--start" in plain(approve.output) + assert "--promote" in plain(complete.output) + + # --- pre-label --------------------------------------------------------------- diff --git a/tests/cli/test_ingest_commands.py b/tests/cli/test_ingest_commands.py index 06173912..fa8cd475 100644 --- a/tests/cli/test_ingest_commands.py +++ b/tests/cli/test_ingest_commands.py @@ -114,6 +114,52 @@ def test_ingesting_the_same_folder_twice_creates_no_new_assets(root: Path, tmp_p assert _sources(root) == [SourceKind.IMAGE_DIRECTORY] +# --- --start: ingest, approve, start, in one line ---------------------------- + + +def _state(root: Path, name: str, batch: str) -> str: + listed = {row["id"]: row for row in payload(root, "batch", "list", "-p", name)["items"]} + return str(listed[batch]["state"]) + + +def test_start_opens_the_batch_it_filled_and_reports_every_step(root: Path, tmp_path: Path) -> None: + result = run(root, "ingest", str(stills(tmp_path)), "-p", "road-signs", "--start") + + assert result.exit_code == 0, result.output + batch = result.stdout.strip() + assert "\n" not in batch + assert "Ingested 6 new and 0 already-known assets" in result.stderr + assert "Approved batch 'incoming' against schema version 1, in 1 job(s)." in result.stderr + assert f"Batch {batch} is now in_annotation." in result.stderr + assert _state(root, "road-signs", batch) == "in_annotation" + + +def test_start_json_prints_the_started_batch(root: Path, tmp_path: Path) -> None: + document = payload(root, "ingest", str(stills(tmp_path)), "-p", "road-signs", "--start") + assert document["state"] == "in_annotation" + assert document["schema_version"] == 1 + assert document["asset_count"] == 6 + assert _state(root, "road-signs", document["id"]) == "in_annotation" + + +def test_start_without_a_schema_names_the_step_that_refused_and_the_draft_it_left( + root: Path, tmp_path: Path +) -> None: + """The ingest has already committed when approve refuses, so the batch exists + and the output has to say so — the refusal's sentence alone would leave a + reader guessing whether anything was written.""" + ok(root, "project", "create", "bare") + + result = run(root, "ingest", str(stills(tmp_path)), "-p", "bare", "--start") + + assert result.exit_code == 1, result.output + assert result.stdout == "" + assert "Error:" in result.stderr + listed = payload(root, "batch", "list", "-p", "bare")["items"] + assert [b["state"] for b in listed] == ["draft"] + assert f"The approve step refused; batch {listed[0]['id']} is draft." in result.stderr + + # --- refusals Click has to make ---------------------------------------------- From 2df40b14958c153d71d2cc41565c9055d0962684 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:31:55 -0700 Subject: [PATCH 2/4] feat(mcp): start on approve_batch and promote on complete_batch approve_batch(start=True) is start_batch in the same call and complete_batch(promote=True) is promote_batch in the same call. Each answer reports what it did: started says whether the batch was opened, promoted counts the assets that entered the dataset, so a move is never an invisible side effect. Approval and completion each commit before the next step is attempted, and a refusal leaves the batch where the first step put it. The tool reference is regenerated from the served listing. --- docs/content/mcp-tools.md | 4 +- src/visionset/mcp/batches.py | 44 ++++++++++++-- tests/mcp/test_batch_tools.py | 108 ++++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 8 deletions(-) diff --git a/docs/content/mcp-tools.md b/docs/content/mcp-tools.md index ca0baf9b..15d78d1a 100644 --- a/docs/content/mcp-tools.md +++ b/docs/content/mcp-tools.md @@ -31,7 +31,7 @@ error envelope, and the three gate words. | `backfill_thumbnails` | `project` | Render the previews that are missing for a project's assets. | | `list_batches` | `project` | List a project's batches with where each one's assets have got to. | | `get_batch` | `batch_id` | Read one batch: its state, its schema pin, its progress and its jobs. | -| `approve_batch` | `batch_id`, `jobs_of`? | Freeze a batch, pin the project's active schema, and cut it into jobs. | +| `approve_batch` | `batch_id`, `jobs_of`?, `start`? | Freeze a batch, pin the project's active schema, and cut it into jobs. | | `start_batch` | `batch_id` | Open an approved batch for annotation. | | `get_pre_label_plan` | `batch_id`, `connection`, `geometries`? | Which classes a pre-labeling run of that connection over this batch would ask about, which it would leave out, and what shapes it would write. | | `pre_label_batch` | `batch_id`, `connection`, `minimum_confidence`?, `replace_model_labels`?, `geometries`? | Ask a model to label every untouched asset in a batch, one run per open job. | @@ -51,7 +51,7 @@ error envelope, and the three gate words. | `delete_annotations` | `job_id`, `annotation_ids` | Remove annotations from a job. All succeed together or none are removed. | | `set_asset_progress` | `job_id`, `asset_id`, `progress` | Record where one asset of a job has got to, without writing annotations. | | `complete_job` | `job_id` | Close a job, once every one of its assets has been settled. | -| `complete_batch` | `batch_id` | Close a batch, once every one of its jobs is complete. | +| `complete_batch` | `batch_id`, `promote`? | Close a batch, once every one of its jobs is complete. | | `promote_batch` | `batch_id` | Move a completed batch's finished assets into the project's dataset. | | `create_correction_batch` | `batch_id`, `name`, `asset_ids`? | Start a draft batch that corrects a completed one. | | `dataset_stats` | `project` | Count what is in a project's dataset, class by class. | diff --git a/src/visionset/mcp/batches.py b/src/visionset/mcp/batches.py index a6f2d845..a7c32aa3 100644 --- a/src/visionset/mcp/batches.py +++ b/src/visionset/mcp/batches.py @@ -250,6 +250,15 @@ def approve_batch( ), ), ] = None, + start: Annotated[ + bool, + Field( + description=( + "Also open the batch for annotation once it is approved — `start_batch` in " + "the same call. Omit to leave it `approved` and start it later." + ) + ), + ] = False, ) -> dict[str, Any]: """Freeze a batch, pin the project's active schema, and cut it into jobs. @@ -259,15 +268,22 @@ def approve_batch( 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. + has no schema at all — and a refusal leaves the batch a draft. Pass `start` + to open it for work in the same call; without it, call `start_batch` next. + `started` in the answer says whether that happened. Approval is committed + before the start is attempted, so a refused start leaves an `approved` + batch that `start_batch` can still open. """ # `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) + batches = BatchService(workspace) + approved = batches.approve(identifier(batch_id, what="batch_id"), partition) + if start: + batches.start(approved.id) + return {**_batch_payload(workspace, approved.id), "started": start} def start_batch(batch_id: BatchRef) -> dict[str, Any]: @@ -618,16 +634,32 @@ def repin_batch( return _batch_payload(workspace, repinned.id) -def complete_batch(batch_id: BatchRef) -> dict[str, Any]: +def complete_batch( + batch_id: BatchRef, + promote: Annotated[ + bool, + Field( + description=( + "Also move the finished assets into the dataset once the batch is closed — " + "`promote_batch` in the same call. Omit to promote later." + ) + ), + ] = False, +) -> 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. + what `promote_batch` requires; pass `promote` to make that call here, and + `promoted` in the answer counts the assets that entered the dataset — zero + when nothing was asked to move, and zero again when the trunk already held + them all, since promotion is a union. Completion is committed before the + promotion is attempted. """ with opened_workspace() as workspace: completed = BatchService(workspace).complete(identifier(batch_id, what="batch_id")) - return _batch_payload(workspace, completed.id) + entered = DatasetService(workspace).promote(completed.id, actor=_ACTOR) if promote else [] + return {**_batch_payload(workspace, completed.id), "promoted": len(entered)} def create_correction_batch( diff --git a/tests/mcp/test_batch_tools.py b/tests/mcp/test_batch_tools.py index a06b275c..2deeb848 100644 --- a/tests/mcp/test_batch_tools.py +++ b/tests/mcp/test_batch_tools.py @@ -18,6 +18,7 @@ ingested, open_batch, payload, + project, schema, ) @@ -110,6 +111,113 @@ def test_a_batch_cannot_be_completed_while_a_job_is_outstanding( assert error(call("complete_batch", batch_id=batch_id))["message"] +# --- approve with start, complete with promote -------------------------------- + + +def test_approving_with_start_opens_the_batch_and_says_so( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=2) + + approved = payload(call("approve_batch", batch_id=batch_id, start=True)) + + assert approved["state"] == "in_annotation" + assert approved["started"] is True + assert approved["schema_version"] == 1 + assert [j["state"] for j in approved["jobs"]] == ["pending"] + assert payload(call("get_batch", batch_id=batch_id))["state"] == "in_annotation" + + +def test_approving_without_start_reports_that_nothing_was_started( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id = ingested(monkeypatch, tmp_path, count=2) + approved = payload(call("approve_batch", batch_id=batch_id)) + assert approved["state"] == "approved" + assert approved["started"] is False + + +def test_approving_with_start_and_no_schema_refuses_and_leaves_a_draft( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = project(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=1) + batch_id = payload(call("ingest", project=named, path=str(tmp_path / "incoming")))["batch_id"] + + refusal = error(call("approve_batch", batch_id=batch_id, start=True)) + + assert "schema" in refusal["message"] + assert payload(call("get_batch", batch_id=batch_id))["state"] == "draft" + + +def _finished(job_id: str) -> None: + """Every asset of the job marked ``annotated`` and the job closed.""" + for asset in payload(call("next_pending_assets", job_id=job_id, count=100))["items"]: + payload( + call("set_asset_progress", job_id=job_id, asset_id=asset["id"], progress="annotated") + ) + payload(call("complete_job", job_id=job_id)) + + +def test_completing_with_promote_fills_the_trunk_and_counts_what_moved( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id, job_id = open_batch(monkeypatch, tmp_path, count=2) + _finished(job_id) + + completed = payload(call("complete_batch", batch_id=batch_id, promote=True)) + + assert completed["state"] == "completed" + assert completed["promoted"] == 2 + assert completed["promoted_asset_count"] == 2 + assert payload(call("promote_batch", batch_id=batch_id)) == {"items": [], "total": 0} + + +def test_completing_without_promote_moves_nothing_into_the_trunk( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id, job_id = open_batch(monkeypatch, tmp_path, count=2) + _finished(job_id) + + completed = payload(call("complete_batch", batch_id=batch_id)) + + assert completed["state"] == "completed" + assert completed["promoted"] == 0 + assert completed["promoted_asset_count"] == 0 + + +def test_completing_with_promote_over_assets_already_in_the_trunk_counts_zero( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Promotion has no refusal of its own once `complete` has succeeded — `promote` + needs only `completed` — so the outcome left to pin is the idempotent one: a + second batch over assets the trunk already holds closes, and `promoted` is + zero rather than an error.""" + named, first, job_id = open_batch(monkeypatch, tmp_path, count=2) + _finished(job_id) + payload(call("complete_batch", batch_id=first, promote=True)) + again = payload(call("ingest", project=named, path=str(tmp_path / "incoming"))) + second = str(again["batch_id"]) + opened = payload(call("approve_batch", batch_id=second, start=True)) + _finished(str(opened["jobs"][0]["id"])) + + completed = payload(call("complete_batch", batch_id=second, promote=True)) + + assert completed["state"] == "completed" + assert completed["promoted"] == 0 + assert completed["promoted_asset_count"] == 2 + + +def test_completing_with_promote_while_a_job_is_outstanding_promotes_nothing( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _, batch_id, _ = open_batch(monkeypatch, tmp_path, count=2) + assert error(call("complete_batch", batch_id=batch_id, promote=True))["message"] + read = payload(call("get_batch", batch_id=batch_id)) + assert read["state"] == "in_annotation" + assert read["promoted_asset_count"] == 0 + + def test_listing_batch_assets_names_the_job_each_belongs_to( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: From 1da0a4729abcca9c90c9c473eec625804fa206a7 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:31:55 -0700 Subject: [PATCH 3/4] docs: describe the composed lifecycle forms The CLI reference, the MCP guide, the batches page and the tutorial name approve --start, complete --promote and ingest --start, and the start and promote parameters of the MCP tools. The tutorial opens a batch with the one-command form and keeps the two-command walk as the explicit variant. --- docs/content/batches.md | 12 +++++++++--- docs/content/cli.md | 28 +++++++++++++++++++++------- docs/content/mcp.md | 17 +++++++++++++++-- docs/content/tutorial.md | 15 +++++++++++---- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/docs/content/batches.md b/docs/content/batches.md index 98f07394..b7923848 100644 --- a/docs/content/batches.md +++ b/docs/content/batches.md @@ -506,15 +506,21 @@ otherwise loop, which is the shape `SchemaChangeWouldOrphan` already argues for. ```bash visionset batch list --project road-signs -visionset batch approve "$BATCH" --jobs-of 100 +visionset batch approve "$BATCH" --jobs-of 100 [--start] visionset batch start "$BATCH" visionset batch pre-label "$BATCH" CONNECTION [--minimum-confidence FLOAT] [--replace-model-labels] [--geometry SHAPE ...] -visionset batch complete "$BATCH" +visionset batch complete "$BATCH" [--promote] visionset batch promote "$BATCH" ``` Each is one service call, and the listing carries the progress counts because a batch's name and -state do not say whether anybody has started on it. +state do not say whether anybody has started on it. `--start` and `--promote` are the next +command made in the same run - `approve` then `start`, `complete` then `promote` - and +`ingest --start` is `approve` (one job) then `start` on the batch it filled. No transition is +added for them: `start` needs only `approved` and `promote` only `completed`, so the second call +is legal whenever the first succeeded, and each commits on its own. A refused second step +leaves the first step's state in place and the output names it. The MCP tools carry the same +pair as `start` and `promote` parameters, reported back as `started` and `promoted`. **`--jobs-of N` is `BySize`; with no flag the batch becomes one job.** There is no flag for `BySegments`, and that is a decision rather than an omission: its own contract is that the caller diff --git a/docs/content/cli.md b/docs/content/cli.md index 1b81b2d2..231977a3 100644 --- a/docs/content/cli.md +++ b/docs/content/cli.md @@ -19,12 +19,13 @@ visionset schema draft set FILE --project P [--kind K] [--note TEXT] [--revision visionset schema draft clear --project P [--kind K] visionset schema draft publish --project P [--kind K] [--revision N] [--allow-destructive] -visionset ingest PATH --project P [--fps N] [--range S:E]... [--batch-name NAME] +visionset ingest PATH --project P [--fps N] [--range S:E]... [--batch-name NAME] [--start] visionset batch list --project P -visionset batch approve BATCH_ID [--jobs-of N] +visionset batch approve BATCH_ID [--jobs-of N] [--start] visionset batch pre-label BATCH_ID CONNECTION [--minimum-confidence FLOAT] [--replace-model-labels] [--geometry SHAPE]... visionset project pre-label PROJECT CONNECTION [--batch BATCH_ID]... [--minimum-confidence FLOAT] [--geometry SHAPE]... -visionset batch start|complete|promote BATCH_ID +visionset batch start|promote BATCH_ID +visionset batch complete BATCH_ID [--promote] visionset job list --batch BATCH_ID visionset job next JOB_ID [-n COUNT] @@ -69,11 +70,9 @@ visionset project create road-signs visionset schema apply schema.json --project road-signs BATCH=$(visionset ingest ./incoming --project road-signs) -visionset batch approve "$BATCH" --jobs-of 100 -visionset batch start "$BATCH" +visionset batch approve "$BATCH" --jobs-of 100 --start # …annotate, in the app or through `visionset job mark`… -visionset batch complete "$BATCH" -visionset batch promote "$BATCH" +visionset batch complete "$BATCH" --promote visionset release publish --tag v1.0 --project road-signs --split 0.7,0.15,0.15 visionset release verify v1.0 --project road-signs && \ @@ -367,6 +366,21 @@ and touching ranges merged. The run is **synchronous**, and there is no `promote`. Each maps to the `BatchService` method of the same name, except `promote`, which is `DatasetService.promote` - it takes a *batch* id and derives the dataset, which is why it lives here. +**Two of the walk's steps carry the next one as a flag.** `approve --start` approves and then +starts; `complete --promote` completes and then promotes; `ingest --start` ingests, approves as one +job, and starts. Each is the two commands it names, made one after the other, and nothing new +in the kernel: `start` requires only `approved`, and `promote` only `completed`, so the second +step is legal exactly when the first succeeded. The first step commits before the second is +attempted, and a refusal in the second prints the first step's line, then the refusal as the +single-step command would print it, then a line naming which step refused and the state the +batch is actually in - `The approve step refused; batch … is draft.` - so nothing has to be +guessed from the exit code. The commonest refusal is `ingest --start` into a project with no +schema, which leaves the draft the ingest made. Plain output prints both outcomes; `--json` +prints the batch document the walk ended on, except `complete --promote --json`, which prints +`{"batch": …, "promoted": …}` - the closed batch beside the page of assets that entered the +dataset. Stdout stays the batch id alone in every composed form, so `BATCH=$(visionset ingest +./incoming --project P --start)` hands the next command an open batch. + `pre-label BATCH_ID CONNECTION [--minimum-confidence FLOAT] [--replace-model-labels] [--geometry SHAPE]...` blocks and calls `visionset.inference.pre_label` inline because a terminal has no dispatcher, once per open job of the batch: a run is over one job's assets, so the batch command diff --git a/docs/content/mcp.md b/docs/content/mcp.md index 45aca761..59ebf145 100644 --- a/docs/content/mcp.md +++ b/docs/content/mcp.md @@ -118,14 +118,14 @@ page groups them by what they are for. | `create_batch` | Start a draft over a chosen set of a project's assets. | | `add_batch_assets` | Put assets into a draft. | | `remove_batch_assets` | Take assets out of a draft. Deletes nothing. | -| `approve_batch` | Freeze it, pin the schema, cut it into jobs. | +| `approve_batch` | Freeze it, pin the schema, cut it into jobs. `start: true` opens it for annotation in the same call, and `started` in the answer says whether that happened. | | `start_batch` | Open it for annotation. | | `get_pre_label_plan` | Which classes a run of a connection would ask about, which it would leave out, and what shapes it writes. | | `pre_label_batch` | Ask a model to label every untouched asset of a batch, one run per open job; one outcome per job. Blocks until it is done. | | `pre_label_project` | The same, over every open batch of a project or the named ones; one outcome per open job. Blocks until done. | | `repin_batch` | Move its schema pin onto the current active version. | | `list_batch_assets` | What is in it, paged, with each asset's job and progress. `job_id` narrows it to one job's frames. | -| `complete_batch` | Close it, once every job is complete. | +| `complete_batch` | Close it, once every job is complete. `promote: true` moves the finished assets into the dataset in the same call, and `promoted` in the answer counts them. | | `promote_batch` | Move the finished assets into the dataset. | | `create_correction_batch` | Start a draft that corrects a completed one. | @@ -134,6 +134,19 @@ the other one: picking a subset by hand, which is what the browser's gallery doe agent doing the same work needs. All three are `draft` only, because past approval the batch is already cut into jobs — see [batches.md](batches.md). +`approve_batch` and `complete_batch` each take the next step of the walk as a parameter, on the +`job_started` principle below: a move an agent asked for is made and reported, never made behind +it. `start: true` on `approve_batch` is `start_batch` in the same call, and `promote: true` on +`complete_batch` is `promote_batch` in the same call; `started` and `promoted` in the answers say +what each did, so an agent reading only the answer knows which state the batch is in and how +many assets entered the dataset - `promoted` is zero when nothing was asked to move and zero +again when the trunk already held every asset, since promotion is a union. The first step +commits before the second is attempted: a refusal is the second step's own, and it leaves the +batch where the first step put it - `approved`, or `completed` - which `get_batch` confirms. The +one refusal the composed forms meet in practice is approval's own, a project with no schema, +which leaves the batch a draft. `start_batch` and `promote_batch` are unchanged for a caller that +wants the steps apart. + ### Jobs and annotations | | | diff --git a/docs/content/tutorial.md b/docs/content/tutorial.md index dce04b36..c3305b1d 100644 --- a/docs/content/tutorial.md +++ b/docs/content/tutorial.md @@ -89,6 +89,13 @@ which somebody is going to label. ```bash BATCH= +visionset batch approve "$BATCH" --jobs-of 100 --start +``` + +That is two moves in one command - `approve`, then `start` - and the same batch can be walked in +two commands when you want to look at the jobs before anybody opens them: + +```bash visionset batch approve "$BATCH" --jobs-of 100 visionset batch start "$BATCH" ``` @@ -150,12 +157,12 @@ the batch, then promote: ```bash visionset job complete -visionset batch complete "$BATCH" -visionset batch promote "$BATCH" +visionset batch complete "$BATCH" --promote ``` -`complete` on the batch refuses while any job is still open: "derived" here means *recomputed*, -not automatic. **Promotion** is what moves assets into the trunk - a union against what is already +`--promote` is `batch promote` made right after `batch complete`; the two commands, one after +the other, do the same. `complete` on the batch refuses while any job is still open: "derived" +here means *recomputed*, not automatic. **Promotion** is what moves assets into the trunk - a union against what is already there, so promoting twice adds nothing and re-promoting after a curator removed something puts it back. From ad1d5aa90e6339e8e404b46017ed92a10f1d1108 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 04:23:06 -0700 Subject: [PATCH 4/4] feat(ui): Approve and start, and Complete and promote, as one control each Two lifecycle steps behind one control, offered on the first step's declaration: Approve and start on a draft row of the batch table and on the ingest run card once a run settles, where it is the card's filled control and Open batch steps down; Complete and promote in the gallery header beside Complete, withheld the same way while frames are outstanding. Each sends the two requests in order and stops where the kernel stops it, showing the first step's outcome as a line and a second-step refusal beneath it. Approve and start reads the project's schema itself, since approval pins one and the batch's declaration cannot say whether there is one to pin; without a schema the row keeps its controls and the run card says what approving needs and where to get it. A control keeps its own record of having been pressed, so the line survives the batch's declaration moving on. --- docs/content/ui.md | 28 ++ frontend/app/cycle/cycle.spec.ts | 18 + frontend/app/e2e/gallery.spec.ts | 67 ++- .../ui-core/src/screens/BatchesScreen.tsx | 5 + .../src/screens/ComposedTransitions.tsx | 306 ++++++++++++++ .../ui-core/src/screens/GalleryScreen.tsx | 9 + frontend/ui-core/src/screens/IngestScreen.tsx | 29 +- .../src/screens/composedTransitions.test.tsx | 396 ++++++++++++++++++ 8 files changed, 850 insertions(+), 8 deletions(-) create mode 100644 frontend/ui-core/src/screens/ComposedTransitions.tsx create mode 100644 frontend/ui-core/src/screens/composedTransitions.test.tsx diff --git a/docs/content/ui.md b/docs/content/ui.md index a82c492c..bc5c15b6 100644 --- a/docs/content/ui.md +++ b/docs/content/ui.md @@ -907,6 +907,34 @@ it means typing tuples of UUIDs. Its `kind` is always sent explicitly - a discriminated union's tag emitted by default reads as optional in the schema while pydantic needs it in the dict to pick a variant. +#### Two steps as one + +Two pairs of lifecycle steps are offered as one control, on the screens where a person +takes them together. **Approve and start** takes a draft through `approved` to +`in_annotation`, cut into one job - the common batch; a batch that needs splitting still +goes through *Approve*, whose dialog is where a partition is chosen. It sits on a draft's +row in the batch table, secondary like every row action, and on the ingest run card once +a run settles, where it is the card's one filled control and *Open batch* steps down +beside it. **Complete and promote** closes the batch and moves its work into the trunk; +it sits in the gallery header beside *Complete*, withheld the same way while any frame is +still to do, and says what the press moved in the same sentence *Promote* uses. + +Neither is a new transition. Each control sends the two requests a person could send one +at a time, in order, and stops where the kernel stops it: a refusal on the second step +leaves the batch where the first step put it, and the control says so - the first step's +outcome as a line (*Approved against v3*, *Completed, finishing 1 job*), the second +step's refusal beneath it in the shared vocabulary, never in its place. The line stays +once the batch's own declaration has moved on, because it is the only thing on the +screen that explains the state the batch is now in. + +*Approve and start* is offered only while the project has an active schema. Approval pins +one, and `allowed_actions` cannot say whether there is one to pin: that is a fact about +the project rather than a state of the batch, so the surface reads the schema itself. +Without one, a draft's row keeps its existing controls and the run card says in words +what approving needs, with the way to the schema section beside it, and keeps *Open +batch* filled. Only a schema that does not exist counts as none - any other failure of +that read says nothing about the project, and the card offers what it always did. + #### Deleting a batch, behind `⋯` and at two anchors The one control on either of these screens that ends a batch rather than moving it diff --git a/frontend/app/cycle/cycle.spec.ts b/frontend/app/cycle/cycle.spec.ts index 43603d93..dc90f7ad 100644 --- a/frontend/app/cycle/cycle.spec.ts +++ b/frontend/app/cycle/cycle.spec.ts @@ -404,6 +404,16 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa // Walking back through the project to find the batch instead would be a suite // finding it by another road, which cannot notice that the screen offers none. await expect(page.getByTestId("run-outcome")).toContainText("cycle-batch"); + // The project declared a schema before this ingest, so the card's filled + // control is the composed step — approve as one job and open for work — + // and *Open batch* steps down beside it. The walk keeps to the two-step + // road below, which is the one that exercises the partition dialog. + await expect(page.getByTestId("approve-start-cycle-batch")).toHaveAttribute( + "data-variant", + "primary", + ); + await expect(page.getByTestId("open-batch")).toHaveAttribute("data-variant", "secondary"); + await expect(page.getByTestId("approve-needs-schema")).toHaveCount(0); await page.getByTestId("open-batch").click(); await expect(page).toHaveURL(/\/projects\/[0-9a-f-]+\/batches\/[0-9a-f-]+$/); }); @@ -471,6 +481,11 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa await openProject(page, PROJECT, "batches"); await expect(page.getByTestId("batches-table")).toBeVisible(); await expect(page.getByTestId("batch-cycle-batch")).toContainText("pending approval"); + // A draft row offers the composed step too, secondary like every row action. + await expect(page.getByTestId("approve-start-cycle-batch")).toHaveAttribute( + "data-variant", + "secondary", + ); await page.getByTestId("approve-cycle-batch").click(); await page.getByTestId("approve-submit").click(); @@ -495,6 +510,9 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa // choose from — the job sits flat under the header, and the batch bar is // the page's one bar. await expect(page.getByTestId("start-annotating")).toHaveCount(0); + // The composed closing move sits beside Complete and is withheld the same + // way while every frame is still to do. + await expect(page.getByTestId("complete-promote-cycle-batch")).toBeDisabled(); await expect(page.getByTestId("job-panels")).toHaveCount(0); await expect(page.getByTestId(/^job-header-/)).toHaveCount(0); const workspace = page.getByTestId("job-workspace"); diff --git a/frontend/app/e2e/gallery.spec.ts b/frontend/app/e2e/gallery.spec.ts index c0211d05..8ae8ad90 100644 --- a/frontend/app/e2e/gallery.spec.ts +++ b/frontend/app/e2e/gallery.spec.ts @@ -224,6 +224,9 @@ async function serveApi(page: Page, sent: Request[], options: Options = {}): Pro // What this run has removed, so the listing and the counts move with the // DELETE the same way the server's would. const removed = new Set(); + // What a promotion moved into the trunk, read back by the batch the way the + // server derives `promoted_asset_count` per read. + let promoted = 0; await page.route("**/api/**", async (route) => { const request = route.request(); @@ -371,12 +374,42 @@ async function serveApi(page: Page, sent: Request[], options: Options = {}): Pro asset_count: counts.total - removed.size, progress: counts, allowed_actions: batchActions(current), - promoted_asset_count: 0, + promoted_asset_count: promoted, parent_batch_id: null, pre_label_run: null, } satisfies Wire["BatchOut"], }); } + if (request.method() === "POST" && path === `/batches/${BATCH}/promote`) { + // Only finished work is promoted, and the kernel says so before this + // fixture would — a page that promoted first would meet the real refusal. + if (current !== "completed") { + return route.fulfill({ + status: 409, + json: { code: "BATCH_NOT_COMPLETE", message: `batch 'drive-01' is '${current}'` }, + }); + } + promoted = counts.annotated; + return route.fulfill({ + json: { + items: Array.from({ length: promoted }, (_, at) => ({ + id: `asset-${at}`, + project_id: PROJECT, + modality: "image", + content_hash: `${at}`.padStart(8, "0") + "deadbeef", + width: 1280, + height: 720, + format: "jpeg", + source_id: null, + frame_index: at, + frame_timestamp: null, + thumbnail_hash: null, + ingested_at: "2026-08-01T09:00:00Z", + })), + total: promoted, + } satisfies Wire["AssetPage"], + }); + } if (request.method() === "DELETE" && path === `/batches/${BATCH}/assets`) { // The kernel's own gate, kept rather than stubbed away: membership is // editable in `draft` and nowhere else, so a page that offers this on an @@ -732,6 +765,38 @@ test("the chosen density survives a reload", async ({ page }) => { await expect(page.getByTestId("density")).toHaveValue("0"); }); +// --- two steps as one -------------------------------------------------------- + +test("Complete and promote finishes the job, closes the batch, promotes, and says all three", async ({ + page, +}) => { + const sent: Request[] = []; + await openGallery(page, sent, { settled: true }); + + const composed = page.getByTestId("complete-promote-drive-01"); + await expect(composed).toHaveAttribute("data-variant", "secondary"); + await composed.click(); + + // In order, and the job first: the batch refuses while its job is open. + const line = page.getByTestId("completed-drive-01"); + await expect(line).toHaveText( + "Completed, finishing 1 job. Promoted 3 assets to the dataset. 45 skipped frames stayed out.", + ); + const posts = sent + .filter((request) => request.method() === "POST") + .map((request) => new URL(request.url()).pathname.replace(/^\/api/, "")); + expect(posts).toEqual([ + `/jobs/${JOB}/complete`, + `/batches/${BATCH}/complete`, + `/batches/${BATCH}/promote`, + ]); + // The batch is `completed` now and declares no `complete`, so the button is + // gone — the line that says what it did is not. + await expect(composed).toHaveCount(0); + await expect(page.getByTestId("batch-state")).toHaveText("completed"); + await expect(page.getByTestId("complete-promote-open-dataset-drive-01")).toBeVisible(); +}); + // --- one job, and several ---------------------------------------------------- test("a one-job batch draws its job flat: no accordion, one bar, the job's controls under the header", async ({ diff --git a/frontend/ui-core/src/screens/BatchesScreen.tsx b/frontend/ui-core/src/screens/BatchesScreen.tsx index 541b6dbd..99a238aa 100644 --- a/frontend/ui-core/src/screens/BatchesScreen.tsx +++ b/frontend/ui-core/src/screens/BatchesScreen.tsx @@ -42,6 +42,7 @@ import { Button } from "../primitives/Button"; import { FieldError } from "../primitives/Input"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "../primitives/Table"; import { ApproveDialog, BatchProgressBar, CompleteBatchButton } from "./BatchLifecycle"; +import { ApproveAndStartButton } from "./ComposedTransitions"; import { BATCH_STATE_VARIANT, batchStateLabel } from "./batchState"; import { SchemaForeshadow } from "./SchemaForeshadow"; import { CorrectionButton, CorrectionOf } from "./CorrectionBatch"; @@ -187,6 +188,10 @@ export function BatchesScreen({ it is the only irreversible one — so it goes where you go looking for it, not beside what you press next. */}
+ {/* Beside the row's own step rather than inside its branch: + the control keeps the first step's outcome on screen + after the batch's declaration has moved on. */} + (null); + + const offered = declares(batch, BATCH_ACTION.approve) && schema.isSuccess; + if (!offered && approved === null) return null; + + const pending = approve.isPending || start.isPending; + const label = approve.isPending + ? "Approving…" + : start.isPending + ? "Starting…" + : "Approve and start"; + + return ( +
+ {offered && ( + + )} + {approved !== null && ( + + Approved against v{approved.schema_version} + {start.isSuccess ? ", and open for annotation." : "."} + + )} + {approve.isError && ( + + {refusalProse(approve.error)} + + )} + {start.isError && ( + + {refusalProse(start.error)} + + )} +
+ ); +} + +// --- complete and promote ----------------------------------------------------- + +export interface CompleteAndPromoteButtonProps { + readonly batch: Batch; + readonly projectId: string; + /** The dataset screen, so a promotion can be followed to where it landed. */ + readonly onOpenDataset?: () => void; + readonly className?: string; +} + +/** + * `in_annotation → completed`, then the union into the trunk. Withheld while + * work is outstanding, exactly as *Complete* is beside it — the count is that + * control's to say, once. + */ +export function CompleteAndPromoteButton({ + batch, + projectId, + onOpenDataset, + className, +}: CompleteAndPromoteButtonProps): JSX.Element | null { + const finish = useFinishBatch(batch.id); + const promote = usePromoteBatch(projectId); + const [completed, setCompleted] = useState(null); + + const offered = declares(batch, BATCH_ACTION.complete); + if (!offered && completed === null) return null; + + const outstanding = outstandingWork(batch.progress); + const pending = finish.isPending || promote.isPending; + const label = finish.isPending + ? "Completing…" + : promote.isPending + ? "Promoting…" + : "Complete and promote"; + const summary = promote.isSuccess + ? promotionSummary(promote.data.total, batch.promoted_asset_count, batch.asset_count) + : null; + + return ( +
+ {offered && ( + + )} + {completed !== null && ( + + Completed + {completed.jobsFinished > 0 + ? `, finishing ${completed.jobsFinished} job${completed.jobsFinished === 1 ? "" : "s"}.` + : "."} + {summary === null ? "" : ` ${summary}`} + + )} + {promote.isSuccess && onOpenDataset !== undefined && ( + + )} + {finish.isError && ( + + {refusalProse(finish.error)} + + )} + {promote.isError && ( + + {refusalProse(promote.error)} + + )} +
+ ); +} + +// --- the ingest outcome card's next step ------------------------------------ + +export interface OutcomeNextStepProps { + readonly projectId: string; + readonly batchId: string; + readonly onOpenBatch?: (batchId: string) => void; + /** The project's schema section, for the remedy when there is no schema to pin. */ + readonly onOpenSchema?: () => void; +} + +/** + * What a settled ingest offers over the batch it filled. + * + * The card is the only thing the stepper shows once a run settles, so it is a + * view of its own and may carry one filled control. With an active schema that + * is *Approve and start*, and *Open batch* steps down to secondary; without one + * the card keeps *Open batch* filled and says, in words, what approving needs + * and where to get it. Only a 404 for the schema is "none": any other failure + * of that read says nothing about the project, and the card offers what it + * always did. + */ +export function OutcomeNextStep({ + projectId, + batchId, + onOpenBatch, + onOpenSchema, +}: OutcomeNextStepProps): JSX.Element { + const batch = useBatch(batchId); + const schema = useActiveSchema(projectId); + const noSchema = schema.isError && asApiError(schema.error).code === SCHEMA_NOT_FOUND; + const composable = + schema.isSuccess && batch.data !== undefined && declares(batch.data, BATCH_ACTION.approve); + const [pressed, setPressed] = useState(false); + const composed = composable || pressed; + + return ( + <> + {noSchema && ( +

+ Approving needs a schema.{" "} + {onOpenSchema === undefined ? ( + "Define one" + ) : ( + + )}{" "} + and the batch can start from its row. +

+ )} + {composed && batch.data !== undefined && ( +
setPressed(true)}> + +
+ )} + {onOpenBatch !== undefined && ( + + )} + + ); +} diff --git a/frontend/ui-core/src/screens/GalleryScreen.tsx b/frontend/ui-core/src/screens/GalleryScreen.tsx index d5148c99..2d88f0a1 100644 --- a/frontend/ui-core/src/screens/GalleryScreen.tsx +++ b/frontend/ui-core/src/screens/GalleryScreen.tsx @@ -29,6 +29,7 @@ import { CompleteBatchButton, StartAnnotatingButton, } from "./BatchLifecycle"; +import { CompleteAndPromoteButton } from "./ComposedTransitions"; import { CorrectionButton, CorrectionOf } from "./CorrectionBatch"; import { BatchOverflowMenu } from "./DeleteBatch"; import { DensityControl, Toolbar } from "./GalleryControls"; @@ -446,6 +447,14 @@ function BatchHeader({ {batch !== undefined && batch.state === "in_annotation" && ( )} + {batch !== undefined && ( + + )} {/* The overflow, and it holds exactly one thing. Rename, re-sample and per-batch export were all asked for alongside it and **none has an diff --git a/frontend/ui-core/src/screens/IngestScreen.tsx b/frontend/ui-core/src/screens/IngestScreen.tsx index 1360ef0b..d463e74b 100644 --- a/frontend/ui-core/src/screens/IngestScreen.tsx +++ b/frontend/ui-core/src/screens/IngestScreen.tsx @@ -120,7 +120,7 @@ * run still say which batch holds what it managed to read. */ -import { ArrowLeft, Film, FolderOpen, Image, RefreshCw, RotateCw, TriangleAlert, Upload, X } from "lucide-react"; +import { ArrowLeft, Film, Image, RefreshCw, RotateCw, TriangleAlert, Upload, X } from "lucide-react"; import { useQueryClient } from "@tanstack/react-query"; import { useEffect, @@ -153,6 +153,7 @@ import { SelectValue, } from "../primitives/Select"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "../primitives/Table"; +import { OutcomeNextStep } from "./ComposedTransitions"; import { SchemaForeshadow } from "./SchemaForeshadow"; import { ClipRangeTimeline } from "./ClipRangeTimeline"; import { probeClip, type ClipProbe } from "./clipProbe"; @@ -689,7 +690,9 @@ export function IngestScreen({ > @@ -989,12 +992,16 @@ function Fact({ label, value }: { readonly label: string; readonly value: string function RunCard({ job, + projectId, onOpenBatch, + onOpenSchema, onAgain, onRerun, }: { readonly job: IngestJob | null; + readonly projectId: string; readonly onOpenBatch?: (batchId: string) => void; + readonly onOpenSchema?: () => void; readonly onAgain: () => void; readonly onRerun: () => void; }): JSX.Element { @@ -1078,7 +1085,9 @@ function RunCard({ {(job.state === "completed" || job.state === "failed") && ( @@ -1104,12 +1113,16 @@ function RunCard({ */ function Outcome({ job, + projectId, onOpenBatch, + onOpenSchema, onAgain, onRerun, }: { readonly job: IngestJob; + readonly projectId: string; readonly onOpenBatch?: (batchId: string) => void; + readonly onOpenSchema?: () => void; readonly onAgain: () => void; readonly onRerun: () => void; }): JSX.Element { @@ -1148,13 +1161,15 @@ function Outcome({ )}

+ {batchId !== null && ( + + )}
- {batchId !== null && onOpenBatch !== undefined && ( - - )} {/* Back to step 2, source kept: the same frames into a different batch is a real second run — registration is idempotent and content addressing makes re-reading free. */} diff --git a/frontend/ui-core/src/screens/composedTransitions.test.tsx b/frontend/ui-core/src/screens/composedTransitions.test.tsx new file mode 100644 index 00000000..282aad2c --- /dev/null +++ b/frontend/ui-core/src/screens/composedTransitions.test.tsx @@ -0,0 +1,396 @@ +/** + * Two steps behind one control, and what the control says when the second + * step refuses. + * + * The claim these tests make is about *rendering*, not about the kernel: the + * two requests go out in order, the first step's outcome stays on screen as a + * line, and a refusal of the second step is said beneath it in the shared + * vocabulary rather than in its place. The refusal itself still comes from the + * server — every path here stubs the answer, never the question. + */ + +import { QueryClient } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import { userEvent } from "@testing-library/user-event"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { JSX, ReactNode } from "react"; + +import { ApiProvider } from "../data/ApiProvider"; +import { + ApproveAndStartButton, + CompleteAndPromoteButton, + OutcomeNextStep, +} from "./ComposedTransitions"; +import type { Batch } from "./queries"; +import { batchActions } from "../testing/wire.fixtures.js"; + +const API = "http://visionset.test"; +const PROJECT = "11111111-1111-4111-8111-111111111111"; +const BATCH = "55555555-5555-4555-8555-555555555555"; +const JOB = "77777777-7777-4777-8777-777777777777"; + +type Answer = { status: number; body?: unknown }; +let handlers: ((request: Request) => Answer | undefined)[] = []; +const sent: Request[] = []; +const bodies = new Map(); + +beforeEach(() => { + handlers = []; + sent.length = 0; + bodies.clear(); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(String(input), init); + sent.push(request); + if (request.method !== "GET") bodies.set(request, await request.clone().text()); + for (const handler of handlers) { + const answer = handler(request); + if (answer !== undefined) { + return new Response(JSON.stringify(answer.body ?? null), { + status: answer.status, + headers: { "content-type": "application/json" }, + }); + } + } + return new Response(JSON.stringify({ code: "NO_STUB", message: request.url }), { + status: 500, + headers: { "content-type": "application/json" }, + }); + }); +}); + +afterEach(() => vi.unstubAllGlobals()); + +function on(method: string, pattern: RegExp, answer: Answer): void { + handlers.push((request) => + request.method === method && pattern.test(new URL(request.url).pathname) ? answer : undefined, + ); +} + +function mount(node: ReactNode): JSX.Element { + return ( + + {node} + + ); +} + +const NO_PROGRESS = { + unannotated: 0, + pre_labeled: 0, + annotated: 0, + skipped: 0, + review_pending: 0, + accepted: 0, + total: 0, +}; + +function batch(overrides: Partial = {}): Batch { + const state = overrides.state ?? "draft"; + return { + id: BATCH, + project_id: PROJECT, + name: "drive-01", + state, + schema_version: null, + asset_count: 3, + progress: { ...NO_PROGRESS, unannotated: 3, total: 3 }, + allowed_actions: batchActions(state), + promoted_asset_count: 0, + parent_batch_id: null, + pre_label_run: null, + ...overrides, + } as Batch; +} + +function posts(): string[] { + return sent.filter((r) => r.method === "POST").map((r) => new URL(r.url).pathname); +} + +const SCHEMA = { project_id: PROJECT, version: 3, classes: [] }; +function schemaExists(exists: boolean): void { + on( + "GET", + /\/projects\/[^/]+\/schema$/, + exists + ? { status: 200, body: SCHEMA } + : { status: 404, body: { code: "SCHEMA_NOT_FOUND", message: "none yet" } }, + ); +} + +describe("Approve and start", () => { + const approved = batch({ state: "approved", schema_version: 3 }); + const started = batch({ state: "in_annotation", schema_version: 3 }); + + it("approves as one job, then starts, and says both landed", async () => { + schemaExists(true); + on("POST", /\/approve$/, { status: 200, body: approved }); + on("POST", /\/start$/, { status: 200, body: started }); + render(mount()); + + await userEvent.click(await screen.findByTestId("approve-start-drive-01")); + + await waitFor(() => + expect(posts()).toEqual([`/batches/${BATCH}/approve`, `/batches/${BATCH}/start`]), + ); + // One job for the whole batch, said outright rather than left to a default. + const approve = sent.find((r) => r.method === "POST")!; + expect(JSON.parse(bodies.get(approve) ?? "")).toEqual({ partition: { kind: "single" } }); + const line = await screen.findByTestId("approved-drive-01"); + await waitFor(() => + expect(line.textContent).toBe("Approved against v3, and open for annotation."), + ); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("keeps the approval on screen and says the refusal beneath it when the start is refused", async () => { + schemaExists(true); + on("POST", /\/approve$/, { status: 200, body: approved }); + on("POST", /\/start$/, { + status: 409, + body: { code: "INVALID_TRANSITION", message: "batch 'drive-01' is 'approved'" }, + }); + render(mount()); + + await userEvent.click(await screen.findByTestId("approve-start-drive-01")); + + const said = await screen.findByTestId("approve-start-error-drive-01"); + expect(said.textContent).toContain("This has already moved on"); + expect(said.textContent).not.toContain("INVALID_TRANSITION"); + // The first step's outcome is not replaced by the second step's refusal: + // the batch is `approved`, and the line is what says so. + expect(screen.getByTestId("approved-drive-01").textContent).toBe("Approved against v3."); + }); + + it("says a refused approval in words, and starts nothing", async () => { + schemaExists(true); + on("POST", /\/approve$/, { + status: 409, + body: { code: "EMPTY_BATCH", message: "batch 'drive-01' has no assets" }, + }); + render(mount()); + + await userEvent.click(await screen.findByTestId("approve-start-drive-01")); + + const said = await screen.findByTestId("approve-start-error-drive-01"); + expect(said.textContent).toContain("This batch has no frames"); + expect(posts()).toEqual([`/batches/${BATCH}/approve`]); + expect(screen.queryByTestId("approved-drive-01")).toBeNull(); + }); + + it("is not offered without an active schema to pin", async () => { + schemaExists(false); + render(mount()); + await waitFor(() => expect(sent.some((r) => r.url.endsWith("/schema"))).toBe(true)); + expect(screen.queryByTestId("approve-start-drive-01")).toBeNull(); + }); + + it("is not offered on a batch that does not declare approve", async () => { + schemaExists(true); + render(mount()); + await waitFor(() => expect(sent.some((r) => r.url.endsWith("/schema"))).toBe(true)); + expect(screen.queryByTestId("approve-start-drive-01")).toBeNull(); + }); +}); + +describe("Complete and promote", () => { + const settled = batch({ + state: "in_annotation", + schema_version: 3, + progress: { ...NO_PROGRESS, annotated: 3, total: 3 }, + promoted_asset_count: 3, + }); + const completed = batch({ + state: "completed", + schema_version: 3, + progress: settled.progress, + promoted_asset_count: 3, + }); + const job = { + id: JOB, + batch_id: BATCH, + state: "completed", + asset_count: 3, + assignee: null, + pre_label_run: null, + allowed_actions: [], + }; + const assets = (count: number) => ({ + items: Array.from({ length: count }, (_, at) => ({ + id: `asset-${at}`, + project_id: PROJECT, + modality: "image", + content_hash: `${at}`.padStart(8, "0") + "deadbeef", + width: 1, + height: 1, + format: "jpeg", + source_id: null, + frame_index: at, + frame_timestamp: null, + thumbnail_hash: null, + ingested_at: "2026-08-01T09:00:00Z", + })), + total: count, + }); + + it("completes, then promotes, and says what the press moved", async () => { + on("GET", /\/jobs$/, { status: 200, body: { items: [job], total: 1 } }); + on("POST", /\/complete$/, { status: 200, body: completed }); + on("POST", /\/promote$/, { status: 200, body: assets(3) }); + const openDataset = vi.fn(); + render( + mount( + , + ), + ); + + await userEvent.click(screen.getByTestId("complete-promote-drive-01")); + + await waitFor(() => + expect(posts()).toEqual([`/batches/${BATCH}/complete`, `/batches/${BATCH}/promote`]), + ); + const line = await screen.findByTestId("completed-drive-01"); + await waitFor(() => + expect(line.textContent).toBe("Completed. Promoted 3 assets to the dataset."), + ); + await userEvent.click(screen.getByTestId("complete-promote-open-dataset-drive-01")); + expect(openDataset).toHaveBeenCalledOnce(); + }); + + it("keeps the completion on screen and says the refusal beneath it when promotion is refused", async () => { + on("GET", /\/jobs$/, { status: 200, body: { items: [job], total: 1 } }); + on("POST", /\/complete$/, { status: 200, body: completed }); + on("POST", /\/promote$/, { + status: 409, + body: { code: "BATCH_NOT_COMPLETE", message: "batch 'drive-01' is 'in_annotation'" }, + }); + render(mount()); + + await userEvent.click(screen.getByTestId("complete-promote-drive-01")); + + const said = await screen.findByTestId("complete-promote-error-drive-01"); + expect(said.textContent).toContain("still unfinished"); + expect(said.textContent).not.toContain("BATCH_NOT_COMPLETE"); + expect(screen.getByTestId("completed-drive-01").textContent).toBe("Completed."); + expect(screen.queryByTestId("complete-promote-open-dataset-drive-01")).toBeNull(); + }); + + it("is withheld while frames are outstanding, as Complete is", () => { + render( + mount( + , + ), + ); + expect((screen.getByTestId("complete-promote-drive-01") as HTMLButtonElement).disabled).toBe( + true, + ); + }); + + it("is not offered on a batch that does not declare complete", () => { + render(mount()); + expect(screen.queryByTestId("complete-promote-drive-01")).toBeNull(); + }); +}); + +describe("the ingest outcome's next step", () => { + function draft(): void { + on("GET", /\/batches\/[^/]+$/, { status: 200, body: batch() }); + } + + it("fills Approve and start when the project has a schema, and steps Open batch down", async () => { + schemaExists(true); + draft(); + render( + mount( + , + ), + ); + const composed = await screen.findByTestId("approve-start-drive-01"); + expect(composed.dataset.variant).toBe("primary"); + expect(screen.getByTestId("open-batch").dataset.variant).toBe("secondary"); + expect(screen.queryByTestId("approve-needs-schema")).toBeNull(); + }); + + it("says what approving needs when there is no schema, and keeps Open batch filled", async () => { + schemaExists(false); + draft(); + const openSchema = vi.fn(); + render( + mount( + , + ), + ); + const remedy = await screen.findByTestId("approve-needs-schema"); + expect(remedy.textContent).toContain("Approving needs a schema"); + await userEvent.click(screen.getByTestId("approve-needs-schema-go")); + expect(openSchema).toHaveBeenCalledOnce(); + expect(screen.queryByTestId("approve-start-drive-01")).toBeNull(); + expect(screen.getByTestId("open-batch").dataset.variant).toBe("primary"); + }); + + it("offers what it always did when the schema read fails for another reason", async () => { + on("GET", /\/projects\/[^/]+\/schema$/, { + status: 500, + body: { code: "INTERNAL_ERROR", message: "schema is unreachable" }, + }); + draft(); + render( + mount( + , + ), + ); + await waitFor(() => expect(sent.some((r) => r.url.endsWith("/schema"))).toBe(true)); + expect(screen.queryByTestId("approve-needs-schema")).toBeNull(); + expect(screen.queryByTestId("approve-start-drive-01")).toBeNull(); + expect(screen.getByTestId("open-batch").dataset.variant).toBe("primary"); + }); + + it("keeps the approval line when the batch moves on under it", async () => { + schemaExists(true); + let state: "draft" | "in_annotation" = "draft"; + handlers.push((request) => { + const path = new URL(request.url).pathname; + if (request.method === "GET" && path === `/batches/${BATCH}`) + return { status: 200, body: batch({ state, schema_version: state === "draft" ? null : 3 }) }; + return undefined; + }); + on("POST", /\/approve$/, { status: 200, body: batch({ state: "approved", schema_version: 3 }) }); + handlers.push((request) => { + if (request.method === "POST" && new URL(request.url).pathname.endsWith("/start")) { + state = "in_annotation"; + return { status: 200, body: batch({ state, schema_version: 3 }) }; + } + return undefined; + }); + render( + mount( + , + ), + ); + + await userEvent.click(await screen.findByTestId("approve-start-drive-01")); + + const line = await screen.findByTestId("approved-drive-01"); + await waitFor(() => + expect(line.textContent).toBe("Approved against v3, and open for annotation."), + ); + // The batch no longer declares approve, so the button is gone — and the line + // that says why is still here. + await waitFor(() => expect(screen.queryByTestId("approve-start-drive-01")).toBeNull()); + expect(screen.getByTestId("approved-drive-01")).toBeTruthy(); + }); +});