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-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/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. 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(); + }); +}); 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/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/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 ---------------------------------------------- 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: