diff --git a/.agents/skills/frontend/information-architecture/SKILL.md b/.agents/skills/frontend/information-architecture/SKILL.md index 44211a2b..0b9932b1 100644 --- a/.agents/skills/frontend/information-architecture/SKILL.md +++ b/.agents/skills/frontend/information-architecture/SKILL.md @@ -54,8 +54,8 @@ column carries are in `docs/content/ui/navigation.md`, *Inside a project*. Rules: -- **Annotate enters by job.** The project's `Annotate` control opens the chosen `in_annotation` batch's one job directly (`/jobs/:jobId`) when the batch has exactly one, and the batch gallery otherwise — the gallery's job panel is where a job is chosen, and it is the only door: the gallery header carries the `approved → in_annotation` transition and nothing that opens the editor. With several open batches the dropdown stays batch-level and the same rule applies to the pick. A panel's `Annotate` takes a `pending` job to `in_progress` before opening it; `Continue` and `View` only open. Panel controls are secondary — the header's transition is the page's one filled control. -- **A batch with jobs shows one job's frames at a time.** The gallery is an accordion of jobs with at most one panel open and every panel closable; the open panel's counts, timeline and frames are that job's, and the panel open on arrival is the first job with work left. A draft batch, having no jobs, keeps the flat grid. The rule is observability: a batch-wide grid beneath a per-job control puts two scopes for the same frames on one screen. Nothing about it changes an address — the gallery route is unchanged and the open panel is not in the URL. +- **Annotate enters by job.** The project's `Annotate` control opens the chosen `in_annotation` batch's one job directly (`/jobs/:jobId`) when the batch has exactly one, and the batch gallery otherwise — the gallery is where a job is chosen when there are several, and the job's own door is the only door: the gallery header carries the `approved → in_annotation` transition and nothing that opens the editor. With several open batches the dropdown stays batch-level and the same rule applies to the pick. A job's `Annotate` takes a `pending` job to `in_progress` before opening it; `Continue` and `View` only open. A job's controls are secondary — the header's transition is the page's one filled control while the batch has one, and the navigation column's Annotate once it is open. +- **A batch shows one job's frames at a time, and one job is the batch.** With exactly one job the gallery draws no accordion and no job-level bar: the job's controls — its door, Pre-label, and the assignee as an editable line — sit under the batch header, followed by that job's filter, order, strip and frames, and the batch bar is the page's one bar. From two jobs the gallery is an accordion with at most one panel open and every panel closable; the open panel's counts, timeline and frames are that job's, each header names its assignee, and the panel open on arrival is the first job with work left. A draft batch, having no jobs, keeps the flat grid. The rule is observability: a batch-wide grid beneath a per-job control puts two scopes for the same frames on one screen, and a one-row accordion is a choice with nothing to choose between. Nothing about it changes an address — the gallery route is unchanged and the open panel is not in the URL. - **A correction batch is reached from the batch that needs correcting**, never from a "new batch" form: the gallery header and the Batches row both offer it on a `completed` batch, capability-gated on `create_correction`. The annotator's read-only banner and the gallery's bulk bar *link* to it rather than duplicating it — creating a batch is a curation act, curation lives on the batch view, and a second place batches are made is a second place the rules can drift. - **Dataset is first-class.** It is the product's central object and must be reachable in ≤1 click from any project section. It is never gated behind, or discoverable only through, onboarding UI. Promotion success links onward to it; the gallery links to it once a batch is `completed`. - **The frames blocking a narrowing are a subsection of Schema, not a screen.** They are a *view of* the draft on the editor above them, the same relation version history has to the schema. A row links to **every** batch holding its frame rather than to one: an annotation carries an `asset_id` and no batch, so there is no single annotator address to prefer. The section is omitted entirely when the host wires no batch route, on the rule the Batches section already follows. It shows a window of the frames and states the total as text rather than a "see all": the destination that control would need is a project-wide asset view, and there is none — the count is a property of the proposal, not the length of a list somebody can open. diff --git a/docs/content/ui.md b/docs/content/ui.md index 34a65e30..c68b6710 100644 --- a/docs/content/ui.md +++ b/docs/content/ui.md @@ -726,58 +726,70 @@ virtualizes **rows** (a row is what the browser lays out; virtualizing tiles ins CSS grid means reimplementing the grid). The column count is measured with a `ResizeObserver` rather than guessed from a second breakpoint list. -#### The jobs accordion - -**Once a batch has jobs, its frames are shown per job, and at most one job is open at a -time.** A batch is partitioned, and a person works one part of it: a batch-wide grid beneath a -per-job control would put two scopes for the same frames on one screen, where the control -names a job and the grid, the counts and the timeline answer for everybody. One open panel is -one scope - its chips, its timeline and its tiles all count the same job - which is why -opening a panel closes the one that was open. **Every panel may be closed**: clicking the open -header collapses it, and an accordion with nothing open is the batch read as an index of its -jobs. - -The panel open on arrival is **the first job with work left** - frames still unannotated -or only pre-labeled - and the first job otherwise, so landing on a batch lands on -something to do rather than on a job somebody has finished. Nothing is remembered across -reloads: the rule recomputes from counts that are read anyway, and a remembered panel is -stale the moment somebody else works the job. - -**A collapsed header is the overview**, so a job is picked without opening it: ordinal, -frame count, state, `A of F annotated`, who is working it, and a thin progress bar. The -accordion is rendered only once jobs exist, the same `showsProgress` gate the progress bar -above it uses, so a draft needs no empty state of its own. The assignee is a plain editable -name, not an account: `JobService.assign` gates on nothing, so the control is always -live, and clearing it is the same operation with `null`. A failed read shows its -error instead of the accordion silently vanishing - an empty list and a failed one look -identical to the naive `undefined`-or-zero-items check, and only one of them means -there is nothing to assign. - -**The open panel holds, in order:** the way into the annotator, `Pre-label` and the -assignee; the segment chips, counting *that job* from `GET /jobs/{id}/progress` rather -than the batch; the order select; that job's timeline; and only that job's frames, from -`GET /batches/{id}/assets?job=`. Frame numbers stay batch-wide, because a frame's number -is its place in the batch and renumbering per job would give one picture two names. - -**Thumbnail size is one setting and is rendered outside the panels** - it is a property -of how a grid is read rather than of a job, and it is the same persisted preference -either way. It sits on the batch's own progress row in the header, right of the progress bar -and on the line of its `A of F annotated` readout: the last row above the accordion that is -about the batch rather than about one job. A draft, which has no progress row, keeps it in the -toolbar over its flat grid. The segment filter is the opposite: it belongs to the panel and **resets to -`All` when the open job changes**, because a filter carried across shows an empty panel -for a job with nothing in that state, which reads as a job with no frames. - -**A draft batch has no accordion** - one flat grid, with the membership tools and -selection over it - because it has no jobs, and there is nothing to partition its frames -by. - -Each header is a ` + {start.isError && {refusalProse(start.error)}} + + ); +} + +// --- who has the job ---------------------------------------------------------- + +/** + * Who is working this job. A name, not an account — `JobService.assign` takes a + * plain string and there is no annotator identity to enforce anything against — + * so the control is always live; there is nothing to gate it on. + */ +export function AssigneeEditor({ + batchId, + job, + ordinal, + presentation, +}: { + readonly batchId: string; + readonly job: Job; + readonly ordinal: number; + readonly presentation: "line" | "button"; +}): JSX.Element { + const assign = useAssignJob(batchId, job.id); + const [editing, setEditing] = useState(false); + const [draft, setDraft] = useState(""); + // Escape closes the editor WITHOUT committing, and it does so by unmounting + // the input — which is what fires the blur a naive `onBlur={commit}` would + // then read as "the user tabbed away, save it". This flag is how Escape's own + // blur is told apart from every other one: set immediately before the state + // change that causes it, read (and cleared) by the blur that follows. + const discarding = useRef(false); + + function commit(): void { + const name = draft.trim(); + if (name.length === 0 || name === (job.assignee ?? "")) { + setEditing(false); + return; + } + assign.mutate(name, { onSuccess: () => setEditing(false) }); + } + + function edit(): void { + setDraft(job.assignee ?? ""); + setEditing(true); + } + + const input = ( + setDraft(event.target.value)} + onKeyDown={(event) => { + if (event.key === "Enter") commit(); + if (event.key === "Escape") { + discarding.current = true; + setEditing(false); + } + }} + onBlur={() => { + if (discarding.current) { + discarding.current = false; + return; + } + commit(); + }} + className="w-40" + /> + ); + const clear = job.assignee !== null && !editing && ( + + ); + const error = assign.isError && {refusalProse(assign.error)}; + + if (presentation === "button") { + return ( + <> + {editing ? ( + input + ) : ( + + )} + {clear} + {error} + + ); + } + + return ( +
+
+ ); +} + +// --- toolbar ----------------------------------------------------------------- + +/** + * The five segments, the order, and the density ladder — and it is mounted twice + * with a different half of itself each time. + * + * The segments and the order belong to **one job**; the density belongs to the + * **screen** and is rendered once above the frames. So each mount says which + * half it is, and the props for the other half are the ones it does not pass. + * + * The counts come off a `ProgressCounts` — the job's, or the batch's for a draft + * — and never off the loaded pages: the pages are a window onto a collection that + * can hold fifty thousand, and a filter whose counts described the hundred in + * memory would be a filter that lies about what it filters. `segmentCounts` owns + * the grouping and the argument for it. + */ +export function Toolbar({ + segment = "all", + counts, + onSegment, + sort = "membership", + onSort, + density = DEFAULT_DENSITY, + onDensity, + showSegments, + showDensity = true, +}: { + readonly segment?: Segment; + /** + * Absent while the counts are in flight, and for a job whose progress read was + * refused. The chips and the order select are drawn without their numbers + * rather than withheld — the *numbers* are what is missing, and a panel with no + * way to filter or order is a panel one failed read has made unusable. + */ + readonly counts?: Record; + readonly onSegment?: (next: Segment) => void; + readonly sort?: AssetSort; + readonly onSort?: (next: AssetSort) => void; + readonly density?: number; + readonly onDensity?: (step: number) => void; + /** + * False for a draft, and for the shared size control above the frames. Every + * frame in a draft is in the same state — there is nothing to filter *between* — + * and the counts behind the segments are the documented zeros a batch with no + * jobs reports, so five segments reading `(0)` over a full grid is the screen + * contradicting itself. + */ + readonly showSegments: boolean; + /** + * False beside a job's frames. How big the thumbnails are is one setting for + * the screen, so a copy of it per job would be several answers to one question. + */ + readonly showDensity?: boolean; +}): JSX.Element { + return ( +
+ {showSegments && ( + <> + {/* The row above already wraps, but the control is one joined pill and + is wider than the narrowest viewport on its own. It scrolls within + its own row rather than widening the page — the project navigation's + answer to the same shape — because squashing five state filters + costs more than a scroll does. The padding pair keeps the focus ring + off the scroller's clip. */} +
+
+ {SEGMENTS.map((one) => ( + + ))} +
+
+ + + + )} + + {showDensity && onDensity !== undefined && ( + + )} +
+ ); +} + +/** + * How big the thumbnails are — one setting for the screen, wherever it is drawn. + * + * A native range input, not a Radix slider: `@radix-ui/react-slider` is not a + * dependency and this task adds none. The native control is also keyboard + * operable and announced correctly for free, which a div with a drag handler + * would have had to earn back. + * + * It is its own component because the two paths mount it in different places: a + * draft has it in the toolbar over its flat grid, and a batch with jobs has it on + * the header's progress row, where it is the only thing above the frames that + * is not about one job. + */ +export function DensityControl({ + density, + onDensity, +}: { + readonly density: number; + readonly onDensity: (step: number) => void; +}): JSX.Element { + return ( + + ); +} + +// --- timeline ---------------------------------------------------------------- + +/** + * One cell per loaded frame, coloured by its **exact** state. + * + * Deliberately not the segmented grouping: the toolbar groups because "is there + * work left" is the right thing to filter by, and this strip is the one place you + * can see a whole job's states side by side. Clicking scrolls the grid to that + * frame and marks it, so the eye can find it after the jump. + * + * The time labels read the frames' own `frame_timestamp`, which is the locator + * that survives a re-decomposition, and render nothing rather than deriving + * seconds from a sampling rate that may not exist — a bunch of stills has no fps + * and no timestamps, and a timeline of "0s → 0s" over it would be a fabrication. + */ +export function Timeline({ + assets, + onPick, + highlighted, +}: { + readonly assets: readonly BatchAsset[]; + readonly onPick: (assetId: string) => void; + readonly highlighted: string | null; +}): JSX.Element | null { + if (assets.length === 0) return null; + const start = assets[0]?.frame_timestamp; + const end = assets[assets.length - 1]?.frame_timestamp; + + return ( +
+ + {start === null || start === undefined ? "" : `${Math.round(start)}s`} + +
+ {assets.map((asset) => ( +
+ + {end === null || end === undefined ? "" : `${Math.round(end)}s`} + +
+ ); +} + +/** + * A timeline cell, from the same vocabulary the cards use. + * + * One vocabulary for both, so a colour on the strip and a dot on a card cannot + * come to mean different things — and that vocabulary is *semantic* + * rather than a monochrome ramp off `primary`. A ramp is a quantity: it says + * how far along a frame is and cannot say what kind of state it is in, so + * `accepted` and `annotated` come out the same near-black and + * `review_pending` is that near-black at 40%, which reads as "less annotated" + * rather than as "waiting on somebody". + * + * The colour lives in `batchState.ts`; what stays here is the geometry and the + * highlight ring, which are the strip's own. + */ +function cellClass(progress: AssetProgress | null | undefined, isHighlighted: boolean): string { + const ring = isHighlighted ? " ring-2 ring-ring" : ""; + return `h-full min-w-0 flex-1 ${progressCellClass(progress)}${ring}`; +} + diff --git a/frontend/ui-core/src/screens/GalleryScreen.tsx b/frontend/ui-core/src/screens/GalleryScreen.tsx index 8940ac4f..d5148c99 100644 --- a/frontend/ui-core/src/screens/GalleryScreen.tsx +++ b/frontend/ui-core/src/screens/GalleryScreen.tsx @@ -3,16 +3,15 @@ * * What it holds is the chrome around the frames: the header's provenance line * and the one setting that is about looking rather than working, the thumbnail - * size. **Once the batch has jobs, the frames themselves are inside a job** — see - * `JobPanels`, which owns the accordion, its filters, its timeline and its grid. - * A draft has no jobs, so it keeps the flat grid it always had. + * size. **Once the batch has jobs, the frames themselves belong to a job** — + * flat under the header when the batch has one job, inside an accordion from + * two; see `JobPanels`. A draft has no jobs, so it keeps the flat grid it + * always had. */ import { useState, type JSX, type ReactNode } from "react"; -import { Play } from "lucide-react"; import { readStep, writePref } from "../data/prefs"; -import type { AssetProgress } from "../annotator/jobQueries"; import { Badge } from "../primitives/Badge"; import { Button } from "../primitives/Button"; import { FieldError } from "../primitives/Input"; @@ -32,21 +31,17 @@ import { } from "./BatchLifecycle"; import { CorrectionButton, CorrectionOf } from "./CorrectionBatch"; import { BatchOverflowMenu } from "./DeleteBatch"; -import { JobPanels } from "./JobPanels"; +import { DensityControl, Toolbar } from "./GalleryControls"; +import { JobPanels, SingleJobWorkspace, type JobPanelsProps } from "./JobPanels"; import { PromoteButton } from "./PromoteButton"; -import { BATCH_ACTION, JOB_ACTION, declares } from "../data/capabilities"; +import { BATCH_ACTION, declares } from "../data/capabilities"; import { refusalProse } from "../data/refusals"; import { BATCH_STATE_VARIANT, batchStateLabel, earliestArrival, hasJobs, - progressCellClass, - progressLabel, relativeAge, - SEGMENT_LABEL, - SEGMENTS, - type Segment, } from "./batchState"; import { GALLERY_PAGE_SIZE, @@ -54,11 +49,8 @@ import { useBatchJobs, useBatches, useSource, - useStartJob, - type AssetSort, type Batch, type BatchAsset, - type Job, } from "./queries"; export interface GalleryScreenProps { @@ -222,8 +214,11 @@ export function GalleryScreen({ {showsProgress && jobs.isError && {refusalProse(jobs.error)}} + {/* One job is the batch: its controls and frames sit right under the + header, and the batch bar above is the page's one bar. The accordion + exists from two jobs, where choosing between them is the point. */} {showsProgress && batch.data !== undefined && roster.length > 0 && ( - + ) : ( + + ); +} + // --- header ------------------------------------------------------------------ /** @@ -391,9 +394,9 @@ function BatchHeader({ )} {/* The batch's own next step, answered from `allowed_actions`. The way - *into* the annotator is not here: a batch's frames are partitioned - into jobs, so which frames to open is a question only a job can - answer — see `JobPanels`. + *into* the annotator is not among these: a batch's frames are + partitioned into jobs, so the door is the job's, drawn under this + header with the rest of the job's controls — see `JobPanels`. */} {startsAnnotation && batch !== undefined && } {/* @@ -485,283 +488,5 @@ function BatchHeader({ ); } -// --- the way into a job ------------------------------------------------------- - -/** - * The way into one job. A `pending` job is taken (`start`) and then opened, so - * `in_progress` means somebody has it open; anything else only opens. The label - * says which: Annotate, Continue, View. Never the page's filled control — the - * batch's own step in the header is, and a row of them would be several. - */ -export function StartJobButton({ - batchId, - job, - onOpenJob, -}: { - readonly batchId: string; - readonly job: Job; - readonly onOpenJob: (jobId: string) => void; -}): JSX.Element { - const start = useStartJob(batchId, job.id); - const starts = declares(job, JOB_ACTION.start); - // `Continue` is only ever the word for a job somebody is inside. A `pending` - // job that does not declare `start` — every job of an `approved` batch — is - // not continuable and not startable from here, so it reads as what it is. - const label = starts ? "Annotate" : job.state === "in_progress" ? "Continue" : "View"; - return ( -
- - {start.isError && {refusalProse(start.error)}} -
- ); -} - -// --- toolbar ----------------------------------------------------------------- - -/** - * The five segments, the order, and the density ladder — and it is mounted twice - * with a different half of itself each time. - * - * The segments and the order belong to **one job**, inside its panel; the density - * belongs to the **screen** and is rendered once above the accordion. So each - * mount says which half it is, and the props for the other half are the ones it - * does not pass. - * - * The counts come off a `ProgressCounts` — the job's, or the batch's for a draft - * — and never off the loaded pages: the pages are a window onto a collection that - * can hold fifty thousand, and a filter whose counts described the hundred in - * memory would be a filter that lies about what it filters. `segmentCounts` owns - * the grouping and the argument for it. - */ -export function Toolbar({ - segment = "all", - counts, - onSegment, - sort = "membership", - onSort, - density = DEFAULT_DENSITY, - onDensity, - showSegments, - showDensity = true, -}: { - readonly segment?: Segment; - /** - * Absent while the counts are in flight, and for a job whose progress read was - * refused. The chips and the order select are drawn without their numbers - * rather than withheld — the *numbers* are what is missing, and a panel with no - * way to filter or order is a panel one failed read has made unusable. - */ - readonly counts?: Record; - readonly onSegment?: (next: Segment) => void; - readonly sort?: AssetSort; - readonly onSort?: (next: AssetSort) => void; - readonly density?: number; - readonly onDensity?: (step: number) => void; - /** - * False for a draft, and for the shared size control above the accordion. Every - * frame in a draft is in the same state — there is nothing to filter *between* — - * and the counts behind the segments are the documented zeros a batch with no - * jobs reports, so five segments reading `(0)` over a full grid is the screen - * contradicting itself. - */ - readonly showSegments: boolean; - /** - * False inside a job panel. How big the thumbnails are is one setting for the - * screen, so a copy of it in each panel would be four answers to one question. - */ - readonly showDensity?: boolean; -}): JSX.Element { - return ( -
- {showSegments && ( - <> - {/* The row above already wraps, but the control is one joined pill and - is wider than the narrowest viewport on its own. It scrolls within - its own row rather than widening the page — the project navigation's - answer to the same shape — because squashing five state filters - costs more than a scroll does. The padding pair keeps the focus ring - off the scroller's clip. */} -
-
- {SEGMENTS.map((one) => ( - - ))} -
-
- - - - )} - - {showDensity && onDensity !== undefined && ( - - )} -
- ); -} - -/** - * How big the thumbnails are — one setting for the screen, wherever it is drawn. - * - * A native range input, not a Radix slider: `@radix-ui/react-slider` is not a - * dependency and this task adds none. The native control is also keyboard - * operable and announced correctly for free, which a div with a drag handler - * would have had to earn back. - * - * It is its own component because the two paths mount it in different places: a - * draft has it in the toolbar over its flat grid, and a batch with jobs has it on - * the header's progress row, where it is the only thing above the accordion that - * is not about one job. - */ -function DensityControl({ - density, - onDensity, -}: { - readonly density: number; - readonly onDensity: (step: number) => void; -}): JSX.Element { - return ( - - ); -} - -// --- timeline ---------------------------------------------------------------- - -/** - * One cell per loaded frame, coloured by its **exact** state. - * - * Deliberately not the segmented grouping: the toolbar groups because "is there - * work left" is the right thing to filter by, and this strip is the one place you - * can see a whole batch's states side by side. Clicking scrolls the grid to that - * frame and marks it, so the eye can find it after the jump. - * - * The time labels read the frames' own `frame_timestamp`, which is the locator - * that survives a re-decomposition, and render nothing rather than deriving - * seconds from a sampling rate that may not exist — a bunch of stills has no fps - * and no timestamps, and a timeline of "0s → 0s" over it would be a fabrication. - */ -export function Timeline({ - assets, - onPick, - highlighted, -}: { - readonly assets: readonly BatchAsset[]; - readonly onPick: (assetId: string) => void; - readonly highlighted: string | null; -}): JSX.Element | null { - if (assets.length === 0) return null; - const start = assets[0]?.frame_timestamp; - const end = assets[assets.length - 1]?.frame_timestamp; - - return ( -
- - {start === null || start === undefined ? "" : `${Math.round(start)}s`} - -
- {assets.map((asset) => ( -
- - {end === null || end === undefined ? "" : `${Math.round(end)}s`} - -
- ); -} - -/** - * A timeline cell, from the same vocabulary the cards use. - * - * One vocabulary for both, so a colour on the strip and a dot on a card cannot - * come to mean different things — and that vocabulary is *semantic* - * rather than a monochrome ramp off `primary`. A ramp is a quantity: it says - * how far along a frame is and cannot say what kind of state it is in, so - * `accepted` and `annotated` come out the same near-black and - * `review_pending` is that near-black at 40%, which reads as "less annotated" - * rather than as "waiting on somebody". - * - * The colour lives in `batchState.ts`; what stays here is the geometry and the - * highlight ring, which are the strip's own. - */ -function cellClass(progress: AssetProgress | null | undefined, isHighlighted: boolean): string { - const ring = isHighlighted ? " ring-2 ring-ring" : ""; - return `h-full min-w-0 flex-1 ${progressCellClass(progress)}${ring}`; -} - export { columnsFor } from "./FrameGrid"; export { GALLERY_PAGE_SIZE }; diff --git a/frontend/ui-core/src/screens/JobPanels.tsx b/frontend/ui-core/src/screens/JobPanels.tsx index 19d45a6d..c686dd15 100644 --- a/frontend/ui-core/src/screens/JobPanels.tsx +++ b/frontend/ui-core/src/screens/JobPanels.tsx @@ -1,7 +1,7 @@ /** - * A batch's jobs, as an accordion — at most one of them open, and every one closable. + * A batch's jobs, on the gallery: flat when there is one, an accordion from two. * - * ## Why the frames moved inside a job + * ## Why the frames live inside a job * * A batch's assets are partitioned into jobs at approval, so "which frames am I * working" is a question only a job can answer. While the gallery drew one @@ -10,9 +10,17 @@ * person working job 2 scrolled past job 1's frames to reach their own, and the * strip could not say which job needed them without opening each one. * - * So the grid, the segment counts and the timeline all live in the open panel and - * all read that job. What stays outside is the one setting that is about looking - * rather than about working: the thumbnail size. + * So the grid, the segment counts and the timeline all read one job. What stays + * outside is the one setting that is about looking rather than about working: + * the thumbnail size. + * + * ## One job is the batch + * + * The common batch has one job, and a one-row accordion is a header nobody can + * choose between, a bar repeating the batch's own and a sentence naming a job + * nobody else has. So with exactly one job there is no accordion: the job's + * controls sit under the batch header and its frames follow, and the batch bar + * is the page's one bar. The accordion exists from two jobs. * * ## At most one open, and every panel may be closed * @@ -21,33 +29,20 @@ * accordion read as an index. Which one opens on arrival is `defaultOpenJob`, and * it deliberately waits for **every** job's counts: opening off a half-read map * means opening the wrong job and jumping when the rest land. + * + * A closed panel is unmounted, and what a person chose inside it — the filter, + * the order, the selection — is kept here by job id, so reopening restores it. */ -import { useCallback, useEffect, useRef, useState, type JSX, type KeyboardEvent } from "react"; -import { ChevronDown, ChevronRight, X } from "lucide-react"; +import { useEffect, useState, type JSX, type KeyboardEvent } from "react"; +import { ChevronDown, ChevronRight, User } from "lucide-react"; -import { Button } from "../primitives/Button"; import { Progress } from "../primitives/Feedback"; -import { FieldError, Input } from "../primitives/Input"; +import { FieldError } from "../primitives/Input"; import { refusalProse } from "../data/refusals"; -import { FrameGrid } from "./FrameGrid"; -import { StartJobButton, Timeline, Toolbar } from "./GalleryScreen"; -import { PreLabelButton } from "./PreLabelDialog"; -import { - annotatedShare, - segmentCounts, - segmentProgress, - type Segment, -} from "./batchState"; -import { - useAssignJob, - useJobsProgress, - type AssetSort, - type Batch, - type BatchAsset, - type Job, - type ProgressCounts, -} from "./queries"; +import { DEFAULT_JOB_VIEW, JobWorkspace, patchView, type JobView } from "./GalleryControls"; +import { annotatedShare } from "./batchState"; +import { useJobsProgress, type Batch, type BatchAsset, type Job, type ProgressCounts } from "./queries"; export interface JobPanelsProps { readonly projectId: string; @@ -89,6 +84,77 @@ export function defaultOpenJob( return unfinished?.id ?? jobs[0]?.id ?? null; } +/** + * What each job's frames are being looked at through, remembered across a close. + * + * A patch is applied to the state as it is *then*, not as the caller saw it: the + * grid reports its selection from an effect that can run after a filter change, + * and merging at write time is what keeps that report from undoing the filter. + */ +function useJobViews(): readonly [ + (jobId: string) => JobView, + (jobId: string, patch: Partial) => void, +] { + const [views, setViews] = useState>(new Map()); + const viewOf = (jobId: string): JobView => views.get(jobId) ?? DEFAULT_JOB_VIEW; + const setView = (jobId: string, patch: Partial): void => + setViews((current) => { + const before = current.get(jobId) ?? DEFAULT_JOB_VIEW; + const after = patchView(before, patch); + return after === before ? current : new Map(current).set(jobId, after); + }); + return [viewOf, setView]; +} + +/** + * The one job's controls and frames, with nothing between them and the batch + * header. Its counts are read here, the same way the accordion reads every + * job's, so the segment chips have their numbers. + */ +export function SingleJobWorkspace({ + projectId, + batch, + jobs, + minColumn, + onOpenAsset, + onOpenJob, + onCorrect, + onLoaded, + onSelectionChange, +}: JobPanelsProps): JSX.Element { + const job = jobs[0] as Job; + const { counts: progress, error } = useJobsProgress([job.id]); + const [viewOf, setView] = useJobViews(); + + return ( +
+ {error !== null && {refusalProse(error)}} + { + setView(job.id, patch); + if (patch.selected !== undefined) onSelectionChange?.(patch.selected); + }} + assignee="line" + {...(onOpenAsset === undefined ? {} : { onOpenAsset })} + {...(onOpenJob === undefined ? {} : { onOpenJob })} + {...(onCorrect === undefined ? {} : { onCorrect })} + {...(onLoaded === undefined ? {} : { onLoaded })} + /> +
+ ); +} + export function JobPanels({ projectId, batch, @@ -101,6 +167,7 @@ export function JobPanels({ onSelectionChange, }: JobPanelsProps): JSX.Element { const { counts: progress, error } = useJobsProgress(jobs.map((one) => one.id)); + const [viewOf, setView] = useJobViews(); // `undefined` is "the default has not been applied yet"; `null` is "the person // closed the last panel". The distinction is what stops the default from // reasserting itself: derived every render, it would move the open panel out @@ -120,6 +187,8 @@ export function JobPanels({ function toggle(jobId: string): void { if (jobId !== open) { setOpen(jobId); + // The header reads the open panel's selection; the one opening has its own. + onSelectionChange?.(viewOf(jobId).selected); return; } setOpen(null); @@ -146,22 +215,35 @@ export function JobPanels({ onOpen={() => toggle(job.id)} /> {job.id === open && ( - // Keyed on the job, so the segment, the order and the selection are - // the open job's own rather than the previous one's carried over. - + role="region" + id={`job-panel-${job.id}`} + aria-labelledby={`job-header-${job.id}`} + data-testid={`job-panel-${job.id}`} + className="flex flex-col gap-3 border-t border-border p-3" + > + { + setView(job.id, patch); + if (patch.selected !== undefined) onSelectionChange?.(patch.selected); + }} + assignee="button" + {...(onOpenAsset === undefined ? {} : { onOpenAsset })} + {...(onOpenJob === undefined ? {} : { onOpenJob })} + {...(onCorrect === undefined ? {} : { onCorrect })} + {...(onLoaded === undefined ? {} : { onLoaded })} + /> + )} ))} @@ -202,7 +284,9 @@ function JobHeader({ id={`job-header-${job.id}`} data-testid={`job-header-${job.id}`} aria-expanded={expanded} - aria-controls={`job-panel-${job.id}`} + // Only while the panel exists: a closed panel is unmounted, and an id + // pointing at nothing is a broken reference rather than a closed one. + {...(expanded ? { "aria-controls": `job-panel-${job.id}` } : {})} onClick={onOpen} onKeyDown={moveBetweenHeaders} className={ @@ -227,11 +311,13 @@ function JobHeader({ )} - - {job.assignee ?? "—"} + + ); @@ -267,200 +353,3 @@ function moveBetweenHeaders(event: KeyboardEvent): void { event.preventDefault(); target.focus(); } - -/** - * One job's frames and everything that acts on them. - * - * In order: the door into the annotator and the two things that can be done to - * the job as a whole, then that job's filters, its timeline and its grid. The - * counts behind the segments are the **job's** `ProgressCounts`, never the - * batch's — a chip reading `All (48)` over a job holding twelve is the filter - * lying about what it filters. - */ -function JobPanel({ - projectId, - batch, - job, - ordinal, - minColumn, - counts, - onOpenAsset, - onOpenJob, - onCorrect, - onLoaded, - onSelectionChange, -}: { - readonly projectId: string; - readonly batch: Batch; - readonly job: Job; - readonly ordinal: number; - readonly minColumn: number; - readonly counts: ProgressCounts | undefined; - readonly onOpenAsset?: (asset: BatchAsset) => void; - readonly onOpenJob?: (jobId: string) => void; - readonly onCorrect?: () => void; - readonly onLoaded?: (assets: readonly BatchAsset[]) => void; - readonly onSelectionChange?: (selected: ReadonlySet) => void; -}): JSX.Element { - const [segment, setSegment] = useState("all"); - const [sort, setSort] = useState("membership"); - const [loaded, setLoaded] = useState([]); - const [highlighted, setHighlighted] = useState(null); - const scrollToAsset = useRef<((assetId: string) => void) | null>(null); - - // Both this panel's timeline and the screen's header read the same window, and - // the identity has to be stable: `FrameGrid` re-runs its report whenever this - // changes. - const report = useCallback( - (assets: readonly BatchAsset[]) => { - setLoaded(assets); - onLoaded?.(assets); - }, - [onLoaded], - ); - - return ( -
-
- {onOpenJob !== undefined && ( - - )} - - -
- - - - { - scrollToAsset.current?.(assetId); - setHighlighted(assetId); - }} - /> - - -
- ); -} - -/** - * Who is working this job. A name, not an account — `JobService.assign` takes a - * plain string and there is no annotator identity to enforce anything against — - * so the control is always live; there is nothing to gate it on. - */ -function AssigneeEditor({ - batchId, - job, - ordinal, -}: { - readonly batchId: string; - readonly job: Job; - readonly ordinal: number; -}): JSX.Element { - const assign = useAssignJob(batchId, job.id); - const [editing, setEditing] = useState(false); - const [draft, setDraft] = useState(""); - // Escape closes the editor WITHOUT committing, and it does so by unmounting - // the input — which is what fires the blur a naive `onBlur={commit}` would - // then read as "the user tabbed away, save it". This flag is how Escape's own - // blur is told apart from every other one: set immediately before the state - // change that causes it, read (and cleared) by the blur that follows. - const discarding = useRef(false); - - function commit(): void { - const name = draft.trim(); - if (name.length === 0 || name === (job.assignee ?? "")) { - setEditing(false); - return; - } - assign.mutate(name, { onSuccess: () => setEditing(false) }); - } - - return ( - <> - {editing ? ( - setDraft(event.target.value)} - onKeyDown={(event) => { - if (event.key === "Enter") commit(); - if (event.key === "Escape") { - discarding.current = true; - setEditing(false); - } - }} - onBlur={() => { - if (discarding.current) { - discarding.current = false; - return; - } - commit(); - }} - className="w-40" - /> - ) : ( - - )} - {job.assignee !== null && !editing && ( - - )} - {assign.isError && {refusalProse(assign.error)}} - - ); -} diff --git a/frontend/ui-core/src/screens/batchLifecycle.test.tsx b/frontend/ui-core/src/screens/batchLifecycle.test.tsx index e7e552b4..0a763ff0 100644 --- a/frontend/ui-core/src/screens/batchLifecycle.test.tsx +++ b/frontend/ui-core/src/screens/batchLifecycle.test.tsx @@ -21,7 +21,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { JSX, ReactNode } from "react"; import { ApiProvider } from "../data/ApiProvider"; -import { ApproveDialog } from "./BatchLifecycle"; +import { ApproveDialog, BatchProgressBar } from "./BatchLifecycle"; import type { Batch } from "./queries"; import { batchActions } from "../testing/wire.fixtures.js"; @@ -182,3 +182,37 @@ describe("the approve dialog's refusals", () => { expect(screen.queryByTestId("approve-schema-missing")).toBeNull(); }); }); + +describe("BatchProgressBar", () => { + const COUNTS = { + total: 11, + unannotated: 4, + pre_labeled: 0, + annotated: 7, + skipped: 0, + review_pending: 0, + accepted: 0, + }; + + it("draws annotation as a success fill on a bordered track, above the counts", () => { + render(); + const bar = screen.getByRole("progressbar", { name: "Annotation progress" }); + expect(bar.getAttribute("aria-valuenow")).toBe("64"); + expect(bar.className).toContain("h-2"); + expect(bar.className).toContain("border"); + expect((bar.firstElementChild as Element).className).toContain("bg-success"); + expect(screen.getByText(/7 annotated · 0 skipped · 0 accepted · 4 to do/)).toBeTruthy(); + }); + + it("says a draft is not approved yet rather than counting work it has no jobs for", () => { + render( + , + ); + expect(screen.getByRole("progressbar").getAttribute("aria-valuenow")).toBe("0"); + expect(screen.getByText("Not approved yet")).toBeTruthy(); + expect(screen.queryByText(/to do/)).toBeNull(); + }); +}); diff --git a/frontend/ui-core/src/screens/gallery.test.tsx b/frontend/ui-core/src/screens/gallery.test.tsx index 8acdff7f..14726396 100644 --- a/frontend/ui-core/src/screens/gallery.test.tsx +++ b/frontend/ui-core/src/screens/gallery.test.tsx @@ -2074,9 +2074,10 @@ describe("the job panel's way into the annotator", () => { />, ), ); - // The accordion first: the frames are *inside* a panel now, so there are no - // tiles at all until the job roster and its counts have both landed. - await screen.findByTestId("job-panels"); + // The job's workspace first: the frames belong to the job, so there are no + // tiles at all until the roster and its counts have both landed. One job + // means no accordion — the workspace sits flat under the header. + await screen.findByTestId("job-workspace"); await screen.findByTestId("tile-asset-0"); return openedJob; } @@ -2084,13 +2085,48 @@ describe("the job panel's way into the annotator", () => { it("offers no door in the header once the batch is open", async () => { await openWith("in_annotation", "pending", "unannotated"); expect(screen.queryByTestId("start-annotating")).toBeNull(); - // Pre-label is a job's action now, so every trigger on the page is inside the - // accordion. Asserting a batch-keyed testid is absent would pass on a page - // that still had a header mount, because the testid is keyed by job. - const panels = screen.getByTestId("job-panels"); - const triggers = [...document.querySelectorAll('[data-testid^="pre-label-"]')]; - expect(triggers.length).toBeGreaterThan(0); - expect(triggers.every((node) => panels.contains(node))).toBe(true); + // Pre-label and the door are the job's, so every trigger on the page is in + // the job's workspace and none in the header. Asserting a batch-keyed testid + // is absent would pass on a page that still had a header mount, because the + // testid is keyed by job. + const workspace = screen.getByTestId("job-workspace"); + const header = screen.getByTestId("batch-title").closest("header") as HTMLElement; + const triggers = [ + ...document.querySelectorAll('[data-testid^="pre-label-"], [data-testid^="start-job-"]'), + ]; + expect(triggers.length).toBe(2); + expect(triggers.every((node) => workspace.contains(node))).toBe(true); + expect(triggers.some((node) => header.contains(node))).toBe(false); + }); + + it("draws one job flat: no accordion, no job header, one bar on the page", async () => { + await openWith("in_annotation", "in_progress", "unannotated", "annotated"); + expect(screen.queryByTestId("job-panels")).toBeNull(); + expect(screen.queryByTestId(`job-header-${JOB}`)).toBeNull(); + expect(screen.queryByTestId(`job-row-${JOB}`)).toBeNull(); + expect(screen.getAllByRole("progressbar")).toHaveLength(1); + // The workspace holds, in order, the action row, the filter, the strip and + // the grid — everything the accordion's open panel held, with nothing to + // choose between above it. + const workspace = within(screen.getByTestId("job-workspace")); + expect(workspace.getByTestId(`start-job-${JOB}`)).toBeTruthy(); + expect(workspace.getByTestId("segments")).toBeTruthy(); + expect(workspace.getByTestId("sort-order")).toBeTruthy(); + expect(workspace.getByTestId("timeline")).toBeTruthy(); + expect(workspace.getByTestId("tile-asset-0")).toBeTruthy(); + expect(workspace.getByTestId(`assignee-${JOB}`).textContent).toContain("Unassigned"); + }); + + it("keeps the header's filled control to the batch's own step, and to at most one", async () => { + // Two filled controls on a page are two answers to "what now?"; none on a + // page whose step is the column's Annotate is correct. The job's door is + // never the filled one — the batch's own step is, while it has one. + const filled = (): Element[] => [ + ...document.querySelectorAll('[data-testid="gallery"] [data-variant="primary"]'), + ]; + await openWith("in_annotation", "in_progress", "unannotated"); + expect(filled()).toHaveLength(0); + expect(screen.getByTestId(`start-job-${JOB}`).dataset.variant).toBe("secondary"); }); it("starts a pending job, then opens it", async () => { @@ -2255,12 +2291,21 @@ describe("the gallery header's own next step", () => { expect(screen.queryByTestId("start-annotating")).toBeNull(); }); - it("offers no Start on an in_annotation batch, whose door is in the job's panel", async () => { + it("offers no Start on an in_annotation batch, whose door is the job's", async () => { await openIn("in_annotation", "annotated", "unannotated", "unannotated"); expect(screen.queryByTestId("start-batch")).toBeNull(); expect(screen.queryByTestId("start-annotating")).toBeNull(); }); + it("fills exactly one control while the batch has a step of its own", async () => { + const filled = (): Element[] => [ + ...document.querySelectorAll('[data-testid="gallery"] [data-variant="primary"]'), + ]; + await openIn("approved", "unannotated", "unannotated"); + expect(filled()).toHaveLength(1); + expect(filled()[0]).toBe(screen.getByTestId("start-batch")); + }); + it("performs the batch's own start rather than navigating into the annotator", async () => { let started = false; handlers.push((request) => { @@ -2379,12 +2424,14 @@ describe("the jobs accordion", () => { } /** - * The assignee editor is in the **open panel**, not on every row: a collapsed - * header is an overview and names who has the job, and the control that changes - * that is one of the things opening a panel is for. + * With one job the editor is a line of the action row — "Assigned to Dana" or + * "Unassigned", the name itself the control. With several, the name is on the + * collapsed header and the editor is in the **open panel**, because a header + * is an overview and the control that changes it is one of the things opening + * a panel is for. */ - async function openPanel(): Promise> { - return within(await screen.findByTestId(`job-panel-${JOB}`)); + async function workspace(): Promise> { + return within(await screen.findByTestId("job-workspace")); } it("names each job's assignee on its own header, open or not", async () => { @@ -2405,10 +2452,10 @@ describe("the jobs accordion", () => { // The overview half of the claim: who has a job is legible without opening // it, and an unassigned one says so rather than saying nothing. expect((await screen.findByTestId(`job-row-${JOB}`)).textContent).toContain("Dana Reyes"); - expect(screen.getByTestId(`job-row-${OTHER_JOB}`).textContent).toContain("—"); + expect(screen.getByTestId(`job-row-${OTHER_JOB}`).textContent).toContain("Unassigned"); // ...and the editor is in whichever panel is open, once. - const panel = await openPanel(); + const panel = within(await screen.findByTestId(`job-panel-${JOB}`)); expect(panel.getByRole("button", { name: "Dana Reyes" })).toBeTruthy(); expect(screen.queryAllByLabelText(/Assignee for job/)).toHaveLength(0); }); @@ -2446,8 +2493,8 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.keyboard("Dana Reyes{Enter}"); const said = await panel.findByRole("alert"); expect(said.textContent).toContain("That job is no longer on record."); @@ -2471,13 +2518,14 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.keyboard("Dana Reyes{Enter}"); const put = sent.find((request) => request.method === "PUT"); expect(put).toBeTruthy(); expect(JSON.parse(bodies.get(put!) ?? "")).toEqual({ assignee: "Dana Reyes" }); - expect(await panel.findByText("Dana Reyes")).toBeTruthy(); + expect(await panel.findByRole("button", { name: "Dana Reyes" })).toBeTruthy(); + expect(panel.getByTestId(`assignee-${JOB}`).textContent).toContain("Assigned to"); }); it("commits the typed name on blur, not only on Enter", async () => { @@ -2490,8 +2538,8 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.type(panel.getByLabelText(/Assignee for job/), "Dana Reyes"); await userEvent.tab(); const put = sent.find((request) => request.method === "PUT"); @@ -2507,11 +2555,11 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.type(panel.getByLabelText(/Assignee for job/), "Dana Reyes"); await userEvent.keyboard("{Escape}"); - expect(await panel.findByRole("button", { name: "Assign" })).toBeTruthy(); + expect(await panel.findByRole("button", { name: "Unassigned" })).toBeTruthy(); expect(sent.some((request) => request.method === "PUT")).toBe(false); }); @@ -2523,10 +2571,10 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.tab(); - expect(await panel.findByRole("button", { name: "Assign" })).toBeTruthy(); + expect(await panel.findByRole("button", { name: "Unassigned" })).toBeTruthy(); expect(sent.some((request) => request.method === "PUT")).toBe(false); }); diff --git a/frontend/ui-core/src/screens/jobPanels.test.tsx b/frontend/ui-core/src/screens/jobPanels.test.tsx index a33c7cb5..7149d91c 100644 --- a/frontend/ui-core/src/screens/jobPanels.test.tsx +++ b/frontend/ui-core/src/screens/jobPanels.test.tsx @@ -334,11 +334,13 @@ describe("which panel opens", () => { await screen.findByTestId(`job-panel-${JOB_B}`); // A job that has stopped existing cannot stay open, and holding its id would - // leave the accordion closed over a batch that has jobs. + // leave the screen closed over a batch that has a job. Down to one, there is + // no accordion to hold open at all: the remaining job's frames sit flat. roster = [job(JOB_A, 2)]; await refetch(["batches"]); - expect(await screen.findByTestId(`job-panel-${JOB_A}`)).toBeTruthy(); + expect(await screen.findByTestId("job-workspace")).toBeTruthy(); + expect(screen.queryByTestId("job-panels")).toBeNull(); expect(screen.queryByTestId(`job-header-${JOB_B}`)).toBeNull(); }); @@ -437,7 +439,42 @@ describe("the collapsed header is the overview", () => { stubs(); renderGallery(); const row = await screen.findByTestId(`job-row-${JOB_B}`); - expect(row.textContent).toContain("—"); + expect(row.textContent).toContain("Unassigned"); + }); + + it("points aria-controls at a panel only while that panel exists", async () => { + // A closed panel is unmounted, so an id pointing at it would point at + // nothing; `aria-expanded="false"` with no `aria-controls` is the honest + // shape, and the open header names the region it controls. + stubs(); + renderGallery(); + await screen.findByTestId(`job-panel-${JOB_A}`); + const open = screen.getByTestId(`job-header-${JOB_A}`); + const closed = screen.getByTestId(`job-header-${JOB_B}`); + expect(open.getAttribute("aria-controls")).toBe(`job-panel-${JOB_A}`); + expect(document.getElementById(`job-panel-${JOB_A}`)).not.toBeNull(); + expect(closed.getAttribute("aria-expanded")).toBe("false"); + expect(closed.hasAttribute("aria-controls")).toBe(false); + }); + + it("restores a job's filter and order when its panel reopens", async () => { + stubs(); + renderGallery(); + await screen.findByTestId(`job-panel-${JOB_A}`); + await userEvent.click(screen.getByTestId("segment-done")); + await userEvent.selectOptions(screen.getByTestId("sort-order"), "confidence"); + expect(screen.getByTestId("segment-done").getAttribute("aria-pressed")).toBe("true"); + + await userEvent.click(screen.getByTestId(`job-header-${JOB_B}`)); + await screen.findByTestId(`job-panel-${JOB_B}`); + // The other job starts from the default: what was chosen was chosen for A. + expect(screen.getByTestId("segment-all").getAttribute("aria-pressed")).toBe("true"); + expect((screen.getByTestId("sort-order") as HTMLSelectElement).value).toBe("membership"); + + await userEvent.click(screen.getByTestId(`job-header-${JOB_A}`)); + await screen.findByTestId(`job-panel-${JOB_A}`); + expect(screen.getByTestId("segment-done").getAttribute("aria-pressed")).toBe("true"); + expect((screen.getByTestId("sort-order") as HTMLSelectElement).value).toBe("confidence"); }); it("moves between headers with the arrow keys", async () => { diff --git a/src/visionset/server/models.py b/src/visionset/server/models.py index 52679c4e..cbd7cb6a 100644 --- a/src/visionset/server/models.py +++ b/src/visionset/server/models.py @@ -1227,7 +1227,7 @@ def of( counts: dict[AssetProgress, int], *, promoted: AbstractSet[UUID], - pre_label_run: PreLabelRun | None = None, + pre_label_run: PreLabelRun | None, ) -> Self: return cls( id=batch.id, @@ -1343,9 +1343,10 @@ def of( counts: dict[AssetProgress, int], *, promoted: AbstractSet[UUID], + pre_label_run: PreLabelRun | None, ) -> Self: return cls( - batch=BatchOut.of(change.batch, counts, promoted=promoted), + batch=BatchOut.of(change.batch, counts, promoted=promoted, pre_label_run=pre_label_run), changed=list(change.changed), ) diff --git a/src/visionset/server/routes/assets.py b/src/visionset/server/routes/assets.py index e99458db..e7b6044d 100644 --- a/src/visionset/server/routes/assets.py +++ b/src/visionset/server/routes/assets.py @@ -179,9 +179,16 @@ def list_asset_batches(workspace: WorkspaceDep, project_id: UUID, asset_id: UUID jobs = JobService(workspace) promoted = _promoted(workspace, project_id) found = batches.holding(asset.id) + pre_label_runs = batches.pre_label_runs() return BatchPage( items=[ - BatchOut.of(batch, jobs.batch_progress(batch.id), promoted=promoted) for batch in found + BatchOut.of( + batch, + jobs.batch_progress(batch.id), + promoted=promoted, + pre_label_run=pre_label_runs.get(batch.id), + ) + for batch in found ], total=len(found), ) diff --git a/src/visionset/server/routes/batches.py b/src/visionset/server/routes/batches.py index 23a4fce9..762a454b 100644 --- a/src/visionset/server/routes/batches.py +++ b/src/visionset/server/routes/batches.py @@ -119,6 +119,7 @@ def create_batch(workspace: WorkspaceDep, project_id: UUID, body: BatchCreate) - created, JobService(workspace).batch_progress(created.id), promoted=_promoted(workspace, project_id), + pre_label_run=batches.latest_pre_label_job(created.id), ) @@ -188,22 +189,26 @@ def approve_batch( pin, and an unknown batch is 404 `BATCH_NOT_FOUND`. """ partition = None if body is None else body.to_domain() - batch = BatchService(workspace).approve(batch_id, partition) + batches = BatchService(workspace) + batch = batches.approve(batch_id, partition) return BatchOut.of( batch, JobService(workspace).batch_progress(batch.id), promoted=_promoted(workspace, batch.project_id), + pre_label_run=batches.latest_pre_label_job(batch_id), ) @router.post("/{batch_id}/start", responses=documented(404, 409)) def start_batch(workspace: WorkspaceDep, batch_id: UUID) -> BatchOut: """Open the batch for annotation. Nothing may be written into it before this.""" - batch = BatchService(workspace).start(batch_id) + batches = BatchService(workspace) + batch = batches.start(batch_id) return BatchOut.of( batch, JobService(workspace).batch_progress(batch.id), promoted=_promoted(workspace, batch.project_id), + pre_label_run=batches.latest_pre_label_job(batch_id), ) @@ -289,11 +294,13 @@ def create_correction_batch( parent's pin. That is the point of correcting under a contract that has moved on, and it is the ordinary approval mechanism rather than anything new. """ - created = BatchService(workspace).create_correction(batch_id, body.name, body.asset_ids) + batches = BatchService(workspace) + created = batches.create_correction(batch_id, body.name, body.asset_ids) return BatchOut.of( created, JobService(workspace).batch_progress(created.id), promoted=_promoted(workspace, created.project_id), + pre_label_run=batches.latest_pre_label_job(created.id), ) @@ -643,6 +650,7 @@ def _membership(workspace: WorkspaceDep, change: MembershipChange) -> BatchMembe change, JobService(workspace).batch_progress(change.batch.id), promoted=_promoted(workspace, change.batch.project_id), + pre_label_run=BatchService(workspace).latest_pre_label_job(change.batch.id), ) diff --git a/tests/server/test_wire_models.py b/tests/server/test_wire_models.py index 5eca486d..61a32039 100644 --- a/tests/server/test_wire_models.py +++ b/tests/server/test_wire_models.py @@ -398,8 +398,16 @@ def test_a_batch_publishes_its_lineage() -> None: child = samples.BATCH.model_copy(update={"parent_batch_id": parent}) orphan = samples.BATCH.model_copy(update={"parent_batch_id": None}) - assert BatchOut.of(child, samples.COUNTS, promoted=frozenset()).parent_batch_id == parent - assert BatchOut.of(orphan, samples.COUNTS, promoted=frozenset()).parent_batch_id is None + assert ( + BatchOut.of(child, samples.COUNTS, promoted=frozenset(), pre_label_run=None).parent_batch_id + == parent + ) + assert ( + BatchOut.of( + orphan, samples.COUNTS, promoted=frozenset(), pre_label_run=None + ).parent_batch_id + is None + ) def test_an_annotation_publishes_the_round_that_produced_it() -> None: