diff --git a/.agents/skills/domain/batch-lifecycle/SKILL.md b/.agents/skills/domain/batch-lifecycle/SKILL.md index e85bc038..87e6eb5f 100644 --- a/.agents/skills/domain/batch-lifecycle/SKILL.md +++ b/.agents/skills/domain/batch-lifecycle/SKILL.md @@ -10,16 +10,20 @@ description: The settled domain model for batch, job, and asset-progress lifecyc The kernel tables are authoritative. Quote them; never re-derive them. - `BATCH_TRANSITIONS` (`kernel/domain/batch.py`): `draft → approved → in_annotation → completed`. **One-way. `completed` has no exit.** -- `JOB_TRANSITIONS` (`kernel/domain/task.py`): `pending → in_progress → completed`. +- `JOB_TRANSITIONS` (`kernel/domain/task.py`): `pending → in_progress → completed`. **One-way; `completed` has no exit either.** - `ASSET_PROGRESS_TRANSITIONS` (`kernel/domain/task.py`): `unannotated ↔ annotated`, `annotated|unannotated → skipped → unannotated`, `annotated → review_pending → annotated|accepted`, `accepted` terminal. -- Derived sets: `SETTLED_PROGRESS = {annotated, skipped, accepted}` (doesn't block completion), `PROMOTABLE_PROGRESS = {annotated, accepted}` (`skipped` never promotes). +- Derived sets: `OPEN_JOB_STATES = {pending, in_progress}` (the job states with a move left — it gates writes, decision 2), `SETTLED_PROGRESS = {annotated, skipped, accepted}` (doesn't block completion), `PROMOTABLE_PROGRESS = {annotated, accepted}` (`skipped` never promotes). **All legality checks go through the `require_move` funnel** (`domain/transitions.py`) or a named set consulted beside it. Hand-rolled membership checks outside the funnel are forbidden (the `repin` hand-roll was finding F13 and has been/is being folded in). ## Settled decisions 1. **Forward-only correction; no reopen.** A `completed` batch is immutable as a workflow unit. There is no `completed → *` transition and none will be added. Corrections happen through a **correction batch**: a new batch over a chosen asset set, pinning the active schema at its own approval, carrying lineage to its parent. Decisions #301/#303 ("settled batches re-enterable to edit") are **superseded** — the legitimate intent behind them ("add one more box later") is served by correction batches. UI on a `completed` batch offers view-only entry plus "Create correction batch" (once it exists), never editing. -2. **Annotation writes are gated on progress** (F11, accepted 2026-08): the kernel refuses annotation add/update/delete unless the asset's progress is `unannotated` or `annotated`. Correcting a `skipped`/`review_pending`/`accepted` asset means moving its progress first (where legal) or a correction batch. Silent label-drop at promotion must be impossible. +2. **Annotation writes are gated on progress *and* on the job** (F11, accepted 2026-08; the job dimension #439, shipped in #447): the kernel refuses annotation add/update/delete unless the asset's progress is `unannotated` or `annotated` (`WRITABLE_PROGRESS`, else `AssetNotWritable` / 409 `ASSET_NOT_WRITABLE`) **and** the job it lives in is still open (`OPEN_JOB_STATES`, else `JobFinished` / 409 `JOB_FINISHED`). `JobService.mark` reads the same set, so a finished job freezes progress moves too. Correcting a `skipped`/`review_pending`/`accepted` asset means moving its progress first (where legal) or a correction batch. Silent label-drop at promotion must be impossible. + - **`OPEN_JOB_STATES` is the single source both sides read** — the declaration (`asset_actions`, which takes batch state, job state and progress: three dimensions, none of them optional) and the refusal (`JobService.require_open_job`, called by the three annotation writes and by `mark`). Declaration and refusal cannot disagree because they are not two rules. A finished job's assets therefore declare *nothing*, which is what turns the annotation workspace into a viewer. + - **The batch gate does not imply the job gate.** `JobService.complete` does not complete the batch — `BatchService` derives that separately, when asked — so the ordinary state of a finished job is *inside a batch that is still `in_annotation`*, where the batch gate has nothing to say. Until #439 a completed job went on accepting labels and progress moves, and an MCP test docstring had written the hole down as a rule ("Writing here is legal — the gate is the batch"). Any prose claiming batch state alone gates annotation writes is stale; correct it rather than reasoning from it. + - **Reads pass no lifecycle gate at all**, only membership. A viewer over finished work has to be able to show it. + - Nothing re-opens a job, by decision 1's argument one level down: correcting finished work is a correction batch, never a move. 3. **`completed` batches cannot be deleted** (F12, accepted 2026-08): `BatchService.delete` refuses `completed` regardless of `confirm`. History is not disposable. 4. **Review is a product flow, not an API-only edge** (F24, decided 2026-08): the annotator provides `annotated → review_pending` (submit for review) and the review-side moves (`→ annotated` reject, `→ accepted`). The gallery's "In review" grouping is backed by reachable UI. 5. **Promotion is not a transition.** It is idempotent trunk-union from a `completed` batch; batch state does not change. Its result must be observable (promoted count, trunk membership on a read model) — invisible success is a bug, not a design. @@ -29,6 +33,9 @@ The kernel tables are authoritative. Quote them; never re-derive them. - **None of that is machinery, and that is the point.** An `Annotation` hangs off its `asset_id` and nothing else, so both rounds write into the same set by construction. Do not add supersession links, per-round filtering, or annotation ids on `DatasetMember` — `promote` moves membership and nothing else. - **Seeding is likewise storage, not a copy.** A correction opens on the labels already on the asset. What approval *does* add is `initial_progress`: an asset that already carries labels starts **`annotated`**, not `unannotated`. The rule reads the asset, never the lineage — an ordinary batch over labeled assets is seeded identically. Its accepted consequence is that a fully seeded correction can be completed with no edits. - **The projection is live**: an edit inside an open batch reaches the trunk on save, not on promotion. Releases are unaffected — the manifest is a frozen blob. +9. **Closure is a job-level fact, and the workspace reads it at job level** (#439, shipped in #447). Read-only is a *transition*, not only an entry state: finishing a job flips the open workspace in place — same window, every frame, from the re-read declaration. Two rulings come with it, and both look like untidiness to a later reader: + - **Frame-verb gating is job-level, never frame-level.** `Skip` / `Un-skip` and the flow verb stop rendering when the *job* is closed (or its batch is), not when the *frame* is read-only. A `skipped` frame is read-only per-frame and still needs its `Un-skip` — the one edge back out of `skipped` — and the navigation cluster is measured to one width, so a slot that emptied and refilled as somebody walked a mixed job would move the arrows under their cursor. cf. #423, #416. + - **`complete` is the job's declaration, not the last frame's.** `Finish job` stays reachable on a frame that is itself settled and read-only: a job whose last frame is `annotated` is precisely the job that is ready to finish. Withdrawing it along with the frame's verbs strands the job with no way to close it. ## What is NOT settled (do not improvise) diff --git a/.agents/skills/frontend/ui-capabilities/SKILL.md b/.agents/skills/frontend/ui-capabilities/SKILL.md index f33d868b..12c0c26d 100644 --- a/.agents/skills/frontend/ui-capabilities/SKILL.md +++ b/.agents/skills/frontend/ui-capabilities/SKILL.md @@ -19,7 +19,7 @@ description: Rules for how the VisionSet frontend decides which actions to offer ## Required patterns - **Disabled-with-reason over hidden** for actions absent from `allowed_actions` but meaningful in context: render disabled with a tooltip stating why ("Batch is completed — create a correction batch to edit"). Fully hide only actions that are never meaningful on that screen. -- **Read-only is a mode, not an accident.** Any surface that can open in a state where writes are not permitted (annotator on a non-`in_annotation` batch) must render an explicit read-only mode: visible banner, editing tools disabled, no dirty state possible. "Open and let saves fail" is forbidden. +- **Read-only is a mode, not an accident.** Any surface that can open in a state where writes are not permitted (annotator on a non-`in_annotation` batch, on a **finished job**, or on a settled frame) must render an explicit read-only mode: visible banner, editing tools disabled, no dirty state possible. "Open and let saves fail" is forbidden. **It is also a transition, not only an entry state** — a mutation made *in* the window can close that window's writes, and the mode must then arrive in place: same page, no navigation, no reload, every frame, out of the re-read declaration and never out of a `setState` mirror of the rule. The kernel's three dimensions are in the `batch-lifecycle` skill; the browser's job is to render whichever of them answered. — #439 - **Every mutation call site** answers three questions in code review: where does a refusal render? where does success render? what happens to the rejected promise? If any answer is "nowhere", the change is incomplete. - **A declaration is a cached answer, so invalidate it.** Every mutation that could change what a resource may be asked to do must invalidate that resource's own query — not only its counts or its data. `allowed_actions` goes stale exactly like a number does, and a stale declaration is the cache-side twin of the hand-mirror: the client is again showing something the kernel no longer agrees with. It shipped as a Finish-job button disabled over a job that was finished, because the job's declaration still described a moment when every asset was `unannotated`. — 2026-08 run, T3 - The app-level error boundary and `unhandledrejection` handler are load-bearing; never remove or bypass them. diff --git a/docs/annotations.md b/docs/annotations.md index f3f1b078..af99bba9 100644 --- a/docs/annotations.md +++ b/docs/annotations.md @@ -154,8 +154,9 @@ another object is the service's. The one exception to the rule in [projects.md](projects.md) and [batches.md](batches.md). Deleting a box is the ordinary annotator edit loop — draw it, look at it, take it off again — -not the destruction of a lifecycle entity the way deleting a project or a batch is. The batch -gate is the guard instead: once the work closes, nothing here can touch it at all. +not the destruction of a lifecycle entity the way deleting a project or a batch is. The +lifecycle gates are the guard instead: once the work closes — the batch, or just this job — +nothing here can touch it at all. ## Progress follows the annotations — two edges of it @@ -175,17 +176,23 @@ applies it inside its own transaction, so labels and progress commit together. I `JobService.mark`, which would open a second session and write from it while the first is still open. -## Work only happens inside an open batch +## Work only happens inside an open batch, and inside an open job -Every write requires the job's batch to be `in_annotation`, else `BatchNotInAnnotation` — the -same error `JobService` raises, reached through the same two lookups (`require_job`, -`require_open_batch`) rather than a second copy of the ladder. +Every write requires the job's batch to be `in_annotation`, else `BatchNotInAnnotation`, **and +the job itself to be open** — `OPEN_JOB_STATES`, else `JobFinished` — the same two errors +`JobService` raises, reached through the same three lookups (`require_job`, +`require_open_batch`, `require_open_job`) rather than a second copy of the ladder. -The gate fires **before** the payload is looked at. A write into a closed batch is a bug -whether or not the annotation is also wrong, and hearing about it only sometimes would hide it. +The second gate is not implied by the first and arrived last, in #439. `JobService.complete` +does not complete the batch, so a finished job ordinarily sits inside one that is still +`in_annotation`: the batch gate had nothing to say, and a job whose work was over went on +accepting labels. See [jobs.md](jobs.md) for the set and the reasoning. -Reads are not gated: `get` and `for_asset` work long after the batch closed, because a label -outlives the work that produced it. +The gates fire **before** the payload is looked at. A write into closed work is a bug whether +or not the annotation is also wrong, and hearing about it only sometimes would hide it. + +Reads are not gated: `get` and `for_asset` work long after the batch closed or the job +finished, because a label outlives the work that produced it. ## Over HTTP diff --git a/docs/jobs.md b/docs/jobs.md index de84b948..90f66b1b 100644 --- a/docs/jobs.md +++ b/docs/jobs.md @@ -174,7 +174,7 @@ though the table alone would call it startable. That dimension is exactly what a re-deriving the rules from `JOB_TRANSITIONS` would drop. `complete` is refined by `SETTLED_PROGRESS` as well, which costs nothing: a job carries its own per-asset map. -**Per asset, inside an `in_annotation` batch:** +**Per asset, inside an open job in an `in_annotation` batch:** | Progress | Declares | | --- | --- | @@ -185,10 +185,13 @@ re-deriving the rules from `JOB_TRANSITIONS` would drop. `complete` is refined b | `accepted` | *nothing* | Anywhere else — a draft, an approved batch, a completed one — every asset declares nothing, -because nothing may be written into a batch nobody opened or one that has closed. +because nothing may be written into a batch nobody opened or one that has closed. **A finished +job empties the column the same way**, inside a batch that is still open: `asset_actions` reads +`OPEN_JOB_STATES`, so the table above is what an asset says while its job is `pending` or +`in_progress`, and `completed` is *nothing*, whatever the progress column would otherwise allow. `annotate` is not a progress move: it is the right to add, change or remove labels, which is -`WRITABLE_PROGRESS` and the batch gate together. The five others each name one edge of +`WRITABLE_PROGRESS` and the two lifecycle gates together. The five others each name one edge of `ASSET_PROGRESS_TRANSITIONS`. Two legal edges deliberately have **no** name — `unannotated ↔ annotated`, the pair an annotation appearing or disappearing makes on its own. They are the consequence of `annotate`, which is declared; offering either as its own control would mean diff --git a/docs/mcp-walkthrough.md b/docs/mcp-walkthrough.md index 460c4c1c..d1d43186 100644 --- a/docs/mcp-walkthrough.md +++ b/docs/mcp-walkthrough.md @@ -379,8 +379,9 @@ Three things were deliberately **not** changed: description, so a destructive tool self-authorises in one call. If this surface wants a real gate it belongs in the server's configuration, out of the agent's reach. Post-beta. - [#109](https://github.com/Robomous/VisionSet/issues/109) — whether `start_job` earned its place - at all, given that writes are gated on the batch. **Settled: it did not.** The tool is gone and - every write starts a `pending` job, reporting `job_started`. See + at all, given that writes were gated on the batch and not on the job. **Settled: it did not.** + The tool is gone and every write starts a `pending` job, reporting `job_started` — and #439's + job gate does not bring it back, because `pending` is one of the open states. See [mcp.md](mcp.md#there-is-no-start_job-the-first-write-starts-it). The limits in [mcp.md](mcp.md) were not re-litigated and none of them caused a failure: ingest and diff --git a/docs/mcp.md b/docs/mcp.md index f3920ab8..d75c4238 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -137,9 +137,12 @@ the first move**. Every tool that writes — the three annotation tools, `set_as **`job_started`** in its answer, so the move is a fact you are told rather than one that happens behind you. `job_started` is `false` on every later call. -Only `pending` moves. A job that is already `in_progress` reports no start, a `completed` one is -left alone, and a job whose batch is not `in_annotation` refuses exactly as it always did — the -batch gate is checked first, so a closed batch is not quietly marked as being worked on. +Only `pending` moves. A job that is already `in_progress` reports no start; a `completed` one is +left alone here and then **refused by the write's own gate** — `JobFinished` (409 +`JOB_FINISHED`), since #439 — so the auto-start neither re-opens finished work nor hides the +refusal behind an `InvalidTransition` of its own. A job whose batch is not `in_annotation` +refuses exactly as it always did: the batch gate is checked first, so a closed batch is not +quietly marked as being worked on. `complete_job` starts a job too, which is not redundant: a correction batch cut over already-labeled assets opens fully settled (see [batches.md](batches.md)), so its job can be @@ -150,7 +153,9 @@ both hops still go through the same funnel; the REST API and the CLI keep their because the annotator page is what drives REST and it has always started a job when a human opens one, while a CLI's explicitness is its contract. #109 has the measurements: two of #36's twelve real agent runs labeled a whole job and then had `complete_job` refuse, having had no reason to -start it — writing is gated on the *batch*, so nothing in the loop forced the call until the end. +start it — writing was gated on the *batch* and not on the job, so nothing in the loop forced the +call until the end. #439 has since added a job gate, but it changes none of this: `pending` is an +*open* state, so the first write still starts the job it walks into. ### Datasets, releases and export diff --git a/docs/ui.md b/docs/ui.md index 0bfd56bc..129486bd 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -252,9 +252,17 @@ caught it the moment the review moves landed, at 3 of 3 becoming 2 of 3. #### Read-only is a mode, not an accident The annotator opens as a **viewer** whenever the frame it is showing does not -declare `annotate` — which the kernel derives from both dimensions at once: the -batch must be `in_annotation` *and* the frame's progress must be in -`WRITABLE_PROGRESS`. One question, both causes. +declare `annotate` — which the kernel derives from all three dimensions at once: +the batch must be `in_annotation`, the job must be in `OPEN_JOB_STATES`, *and* +the frame's progress must be in `WRITABLE_PROGRESS`. One question, three causes. + +**And it is a transition, not only a way to open** (#439). Pressing `Finish job` +closes the job under a window that is already open, so the workspace flips to the +viewer *in place* — same page, no navigation, no reload, on every frame of the +job rather than the last one. Nothing on the page computes that: the mutation +invalidates the frames' declarations and the wire's answer has moved, because +`asset_actions` reads the job's state. Before #439 it did not move, and the page +stayed a live editor over work it had just been told was over. Before this it had no such notion. `batchState` reached the page and was consumed only by the two auto-start effects, so a `completed` batch opened a fully live diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index 9f04142e..e60c9266 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -1052,11 +1052,12 @@ function Workspace({ * turns on (audit finding F2). * * `annotate` is the wire's name for *the right to write labels here at all*, - * and the kernel derives it from both dimensions: the batch must be - * `in_annotation` **and** the frame's progress must be one the labels can still - * move with (`WRITABLE_PROGRESS`, which #304 made a real gate rather than a - * convention). So one question answers both "is this batch closed" and "is this - * frame settled", and neither is re-derived here. + * and the kernel derives it from all three dimensions: the batch must be + * `in_annotation`, the job must still be open (`OPEN_JOB_STATES`, #439), **and** + * the frame's progress must be one the labels can still move with + * (`WRITABLE_PROGRESS`, which #304 made a real gate rather than a convention). + * So one question answers "is this batch closed", "is this job finished" and + * "is this frame settled" alike, and none of the three is re-derived here. * * What it replaces: nothing. There was no read-only mode. `batchState` reached * this component and was consumed **only** by the two auto-start effects, so on diff --git a/frontend/ui-core/src/screens/GalleryScreen.tsx b/frontend/ui-core/src/screens/GalleryScreen.tsx index 0af7538e..ab4f119d 100644 --- a/frontend/ui-core/src/screens/GalleryScreen.tsx +++ b/frontend/ui-core/src/screens/GalleryScreen.tsx @@ -603,8 +603,9 @@ function BatchHeader({ * refuses, and a person's work was stranded in a tab. * * Answered from the frames' own declarations rather than from the batch's - * state, because the kernel derives them from both dimensions and this is the - * same question the annotator itself will ask on arrival. Same control either + * state, because the kernel derives them from every dimension it has — the + * batch's state, the job's and the frame's — and this is the same question the + * annotator itself will ask on arrival. Same control either * way — the door does not move — but the word on it is honest about what is * behind it. */ @@ -1153,8 +1154,9 @@ function ProgressDot({ asset }: { readonly asset: BatchAsset }): JSX.Element { * was refused, and the bar reported "0 moved, N refused" with the reason gone. * * Now each target is a frame whose own `allowed_actions` names the move, which - * the kernel derived from both dimensions. On a batch that is not open the lists - * are empty by construction — so instead of two zeroed buttons the bar states the + * the kernel derived from the batch's state, the job's and the frame's alike. On + * a batch that is not open — or, since #439, in a job that has finished — the + * lists are empty by construction — so instead of two zeroed buttons the bar states the * batch-level reason once, and the buttons are **disabled with it**. The * selection survives, because choosing a set of frames is the first half of * making a correction batch out of them. diff --git a/src/visionset/mcp/_autostart.py b/src/visionset/mcp/_autostart.py index f6e02a3d..2ab54f4d 100644 --- a/src/visionset/mcp/_autostart.py +++ b/src/visionset/mcp/_autostart.py @@ -2,12 +2,16 @@ """Taking a job to ``in_progress`` on the first write, so an agent never has to say so. #109, and the evidence is #36's twelve real agent runs: two of them wrote every -label in a job and then had ``complete_job`` refuse, because writing is gated on -the **batch** being ``in_annotation`` and never on the job, so nothing in the loop -forces a start until the very last call. Both recovered — the kernel's sentence +label in a job and then had ``complete_job`` refuse, because writing was gated on +the **batch** being ``in_annotation`` and not on the job, so nothing in the loop +forced a start until the very last call. Both recovered — the kernel's sentence names the reachable state — but the round trip was wasted, and the description fix #36 shipped could only warn about the ceremony, not remove it. +#439 has since given writes a job gate too, and it leaves that reasoning intact: +``OPEN_JOB_STATES`` holds ``pending``, so a job nobody started is still a job +that may be written into. + **This is adapter policy, not a domain change.** ``JOB_TRANSITIONS`` is untouched, ``require_move`` is still the funnel, and the move made here is the same ``JobService.start`` the retired ``start_job`` tool called. What changed is who @@ -26,8 +30,8 @@ Only ``pending`` is moved. A job that is already ``in_progress`` reports no start, and one that is ``completed`` is left alone for the write's own gate to answer — -the guard is a state check rather than a swallowed ``InvalidTransition``, so no -refusal anybody wrote is hidden. +``JobFinished``, since #439 — the guard being a state check rather than a +swallowed ``InvalidTransition``, so no refusal anybody wrote is hidden. """ from __future__ import annotations