From 9827e2c2290ff10aac743a304b2f3f850c0cb102 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Tue, 4 Aug 2026 16:44:10 -0700 Subject: [PATCH 1/2] feat(ui-core): a finished batch can be corrected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit gap G6, and the end of the forward-only story. A `completed` batch is immutable as a workflow unit — the kernel gives it no exit and none is coming. The product's answer to "this frame is wrong" had been a dead end dressed three different ways: an annotator that opened fully editable and refused every save (#306 made it a viewer), a bulk bar whose buttons were live and whose every request 409'd (#305 disabled them with a reason), and a sentence naming a correction batch that nothing could create. Each of those named the route onward deliberately, on the grounds that it beats a friendlier lie. This is what they were waiting for. **Scope is a choice and the default is the whole batch.** "Correct this batch" is the ordinary ask, so `all` sends no `asset_ids` at all — the server's own default is the parent's whole membership, and re-listing forty-eight ids to say so would be the client telling the API something it already knows. `selection` is the other ordinary ask, and it is offered only when there *is* one: a scope choice whose second option covers nothing is a choice between doing something and doing nothing. There is deliberately no "filtered set" option. The gallery's segments are a *view*, and a correction cut from whatever happens to be filtered at the moment of pressing is a batch nobody can describe afterwards — where a selection is a thing somebody chose. `defaultCorrectionName` is pure and exported because it is the part with a decision in it: a dialog that opened blank would make the common case cost a sentence of typing, and numbering keeps a chain readable in a listing. The count comes from the caller, which is already holding the project's batch listing. `CorrectionOf` renders lineage as one hop — what this batch corrects, not how deep the chain goes, which is the honest reading of the one field there is. It renders nothing without a parent: "not a correction of anything" is most batches, and a badge saying so on every one would be noise on the many to inform the few. --- frontend/ui-core/src/data/capabilities.ts | 1 + frontend/ui-core/src/index.ts | 7 + .../ui-core/src/screens/CorrectionBatch.tsx | 309 ++++++++++++++++++ frontend/ui-core/src/screens/queries.ts | 45 +++ 4 files changed, 362 insertions(+) create mode 100644 frontend/ui-core/src/screens/CorrectionBatch.tsx diff --git a/frontend/ui-core/src/data/capabilities.ts b/frontend/ui-core/src/data/capabilities.ts index e00c36e8..97cc5507 100644 --- a/frontend/ui-core/src/data/capabilities.ts +++ b/frontend/ui-core/src/data/capabilities.ts @@ -64,6 +64,7 @@ export const BATCH_ACTION = { complete: "complete", repin: "repin", promote: "promote", + createCorrection: "create_correction", editMembership: "edit_membership", delete: "delete", } as const satisfies Record; diff --git a/frontend/ui-core/src/index.ts b/frontend/ui-core/src/index.ts index 3d814994..c61810cb 100644 --- a/frontend/ui-core/src/index.ts +++ b/frontend/ui-core/src/index.ts @@ -197,6 +197,13 @@ export { BatchesScreen, type BatchesScreenProps } from "./screens/BatchesScreen. export { GalleryScreen, type GalleryScreenProps } from "./screens/GalleryScreen.js"; export { ApproveDialog, BatchProgressBar } from "./screens/BatchLifecycle.js"; export { PromoteButton, promotionSummary, type PromoteButtonProps } from "./screens/PromoteButton.js"; +export { + CorrectionButton, + CorrectionOf, + defaultCorrectionName, + type CorrectionButtonProps, + type CorrectionScope, +} from "./screens/CorrectionBatch.js"; export { batchStateLabel, segmentCounts, diff --git a/frontend/ui-core/src/screens/CorrectionBatch.tsx b/frontend/ui-core/src/screens/CorrectionBatch.tsx new file mode 100644 index 00000000..55b4cdbf --- /dev/null +++ b/frontend/ui-core/src/screens/CorrectionBatch.tsx @@ -0,0 +1,309 @@ +/** + * Correcting a batch that is finished — audit gap G6, and the end of the + * forward-only story. + * + * ## What this replaces + * + * Nothing, which is the point. A `completed` batch is immutable as a workflow + * unit: the kernel gives it no exit and none is coming. The product's answer to + * "this frame is wrong" had been a dead end dressed three different ways — an + * annotator that opened fully editable and refused every save (#306 made it a + * viewer), a bulk bar whose buttons were live and whose every request 409'd + * (#305 disabled them with a reason), and a sentence naming a correction batch + * that nothing could create. + * + * Each of those now says the same thing and, from here, *points at the same + * control*. That is the difference between a refusal and a next step, and it is + * the whole reason those two tasks left the sentence in place rather than + * inventing a friendlier lie. + * + * ## Scope is a choice, and the default is the whole batch + * + * "Correct this batch" is the ordinary ask, so `all` is the default and sends no + * `asset_ids` at all — the server's own default is the parent's whole + * membership, and re-listing forty-eight ids to say so would be this screen + * telling the API something it already knows. + * + * `selection` exists because the other ordinary ask is *the three frames + * somebody found wrong*, and the gallery already has a selection to hand. It is + * offered only when there is one: a scope choice with an empty option is a + * choice between doing something and doing nothing. + * + * There is deliberately no "filtered set" option. The gallery's segments are a + * *view*, and a correction cut from whatever happens to be filtered at the + * moment of pressing is a batch nobody can describe afterwards — where a + * selection is a thing somebody chose. + */ + +import { GitBranch } from "lucide-react"; +import { useState, type JSX } from "react"; + +import { BATCH_ACTION, declares } from "../data/capabilities"; +import { refusalProse } from "../data/refusals"; +import { Button } from "../primitives/Button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogTitle, +} from "../primitives/Dialog"; +import { FieldError, FieldHint, Input, Label } from "../primitives/Input"; +import { useCreateCorrection, type Batch } from "./queries"; + +/** What a correction covers. `all` sends nothing and lets the server default. */ +export type CorrectionScope = "all" | "selection"; + +/** + * What to call a correction of this batch, before anybody types anything. + * + * Pure and exported because it is the part with a decision in it: a name is + * required, and a dialog that opened blank would make the common case — "yes, + * correct this, that is all I meant" — cost a sentence of typing. Numbering by + * how many corrections already exist keeps a chain readable in a listing, and + * counting is the caller's job because only it has the listing. + */ +export function defaultCorrectionName(parent: string, existing: number): string { + return existing === 0 ? `${parent} — correction` : `${parent} — correction ${existing + 1}`; +} + +export interface CorrectionButtonProps { + readonly batch: Batch; + readonly projectId: string; + /** + * How many corrections of this batch already exist, for the suggested name. + * + * Passed in rather than counted here: the caller is already holding the + * project's batch listing, and a second request to name a dialog would be a + * request nobody asked for. + */ + readonly existingCorrections?: number; + /** The frames currently selected, when the caller has a selection to offer. */ + readonly selection?: readonly string[]; + /** Where to go once the correction exists. Absent leaves the caller where it is. */ + readonly onOpenBatch?: (batchId: string) => void; + readonly className?: string; + /** + * Drive the dialog from outside, for a caller with a second way in. + * + * The gallery has two — the header button and the bulk bar's "Create one" — + * and two independent dialogs would be two states that can both be true. When + * this is supplied the component is controlled and its own button reports + * through `onOpenChange` rather than to itself. + */ + readonly open?: boolean; + readonly onOpenChange?: (open: boolean) => void; +} + +export function CorrectionButton({ + batch, + projectId, + existingCorrections = 0, + selection, + onOpenBatch, + className, + open, + onOpenChange, +}: CorrectionButtonProps): JSX.Element | null { + const [own, setOwn] = useState(false); + const showing = open ?? own; + const setShowing = onOpenChange ?? setOwn; + + // Capability-gated like every other action in this product: `create_correction` + // is declared exactly while the batch is `completed`, and correcting an open + // batch is not a correction — it is the work, in the batch already there. + if (!declares(batch, BATCH_ACTION.createCorrection)) return null; + + return ( + <> + + setShowing(false)} + {...(selection === undefined ? {} : { selection })} + {...(onOpenBatch === undefined ? {} : { onOpenBatch })} + /> + + ); +} + +function CorrectionDialog({ + batch, + projectId, + existingCorrections, + selection, + open, + onClose, + onOpenBatch, +}: { + readonly batch: Batch; + readonly projectId: string; + readonly existingCorrections: number; + readonly selection?: readonly string[]; + readonly open: boolean; + readonly onClose: () => void; + readonly onOpenBatch?: (batchId: string) => void; +}): JSX.Element { + const create = useCreateCorrection(projectId); + const suggested = defaultCorrectionName(batch.name, existingCorrections); + const [name, setName] = useState(suggested); + const [touched, setTouched] = useState(false); + const [scope, setScope] = useState("all"); + + const chosen = selection ?? []; + const canScopeToSelection = chosen.length > 0; + const effective: CorrectionScope = canScopeToSelection ? scope : "all"; + const value = touched ? name : suggested; + + const submit = (): void => { + create.mutate( + { + batchId: batch.id, + name: value, + // Omitted for `all`, so the server's own default answers — see the + // module docstring. + ...(effective === "selection" ? { assetIds: chosen } : {}), + }, + { + onSuccess: (child) => { + onClose(); + onOpenBatch?.(child.id); + }, + }, + ); + }; + + return ( + !next && onClose()}> + +
+ Create a correction batch + + {batch.name} is completed, and a completed batch is kept as it is. A correction is a + new batch over the same frames that records where it came from. + +
+ +
+
+ + { + setTouched(true); + setName(event.target.value); + }} + /> +
+ +
+ Which frames + + {/* + Offered only when there is a selection. A scope choice whose second + option covers nothing is a choice between doing something and doing + nothing, which is not a choice. + */} + {canScopeToSelection && ( + + )} + + The correction starts as a draft, so its frames can still change. Approving it pins + the project’s current label schema — not the one {batch.name} was judged against. + +
+ + {create.isError && ( + {refusalProse(create.error)} + )} +
+ + + + + + + + ); +} + +/** + * "Correction of X" — lineage, rendered where somebody is looking at the child. + * + * A batch's parent is one hop, not a root pointer: each records the one it was + * cut from, and a reader walks the chain for the origin. So this says *of what* + * and nothing about how deep the chain goes, which is the honest reading of the + * one field there is. + * + * `null` when there is no parent, which is most batches and is not a state worth + * drawing: "not a correction of anything" is the ordinary case, and a badge + * saying so on every batch would be noise on the many to inform the few. + */ +export function CorrectionOf({ + parentName, + onOpenParent, +}: { + readonly parentName: string | undefined; + readonly onOpenParent?: () => void; +}): JSX.Element | null { + if (parentName === undefined) return null; + return ( + + + ); +} diff --git a/frontend/ui-core/src/screens/queries.ts b/frontend/ui-core/src/screens/queries.ts index a4cb88d7..c0620e9a 100644 --- a/frontend/ui-core/src/screens/queries.ts +++ b/frontend/ui-core/src/screens/queries.ts @@ -59,6 +59,7 @@ import { checkListReleases, checkListSchemaVersions, checkListSources, + checkCreateCorrectionBatch, checkPromoteBatch, checkPublishRelease, checkRegisterImageSource, @@ -1001,6 +1002,50 @@ export function useFormats() { }); } +/** + * Cut a draft batch that corrects a completed one. + * + * **The forward-only model's one write.** A completed batch has no exit — the + * kernel gives it none and none is coming — so changing settled work means a new + * batch over the same assets, recording `parent_batch_id` back to the one it + * corrects. Nothing about the parent moves. + * + * `assetIds` omitted means the parent's **whole membership**, which is the + * server's default and the ordinary ask. A subset is the other one. + */ +export function useCreateCorrection(projectId: string) { + const client = useApiClient(); + const queries = useQueryClient(); + return useMutation({ + mutationFn: async (input: { + readonly batchId: string; + readonly name: string; + readonly assetIds?: readonly string[]; + }): Promise => + unwrap( + await client.POST("/batches/{batch_id}/corrections", { + params: { path: { batch_id: input.batchId } }, + body: { + name: input.name, + // Omitted rather than sent empty when the caller wants everything: + // `[]` and "all of them" are the same value on this route, and + // relying on that coincidence would break the moment it stops being + // one. `BatchCreate` already spells the opposite meaning. + ...(input.assetIds === undefined ? {} : { asset_ids: [...input.assetIds] }), + }, + }), + checkCreateCorrectionBatch, + ), + onSuccess: () => { + // A new batch in the project's listing, and the parent's own read moves + // too — nothing on it changed, but a screen deriving "does this have + // corrections" from the listing needs the new row. + void queries.invalidateQueries({ queryKey: ["batches"] }); + void queries.invalidateQueries({ queryKey: ["projects", projectId] }); + }, + }); +} + /** * Promote a completed batch into the trunk. Idempotent — a union, not an append. * From ce5a4b4453c002cc8e57f6b6382b75208ba61279 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Tue, 4 Aug 2026 16:44:41 -0700 Subject: [PATCH 2/2] feat(ui-core): every sentence that named a correction batch now reaches one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four surfaces, and the point is that they agree. **Gallery header** — the screen somebody is on when they find the frame that is wrong. It takes the current selection, so "these three are bad" is one press rather than a second pass inside the new batch. **Batches row** — beside Promote rather than in an overflow menu, because a completed batch has exactly two things left to do and hiding the second is hiding the one somebody is hunting for. The row also renders what it corrects, so a chain stays readable when the table is sorted by something else. **The annotator's read-only banner** links to the gallery rather than opening a dialog, and that is a product call: 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. The annotator says which way is forward and hands the person to the screen that owns it. **The bulk bar's withheld sentence** gains "Create one", and the selection it is already holding becomes the correction's scope. The gallery holds the dialog's open state rather than `CorrectionButton` holding its own, because that screen has two ways in and two independent dialogs would be two states that can both be true. `CorrectionButton` is controlled when a caller supplies `open`, uncontrolled otherwise — which keeps the Batches row a one-liner. Lineage on both sides is derived from the project's batch listing rather than fetched: how many corrections point at this batch and what this batch points at are a filter and a lookup over one array the screen's siblings already request. ## Tests Twelve vitest cases for the control and its lineage — the two scopes and exactly what each sends, the selection scope absent when nothing is selected, navigation to the child, a refusal rendered as prose with the dialog staying open (one a dialog closes over is one nobody reads), and the suggested name. The round-trip runs in `cycle.spec.ts` against a **real server**, because the two claims worth making are about the kernel rather than about a stub: that the parent is genuinely untouched after a correction is cut, and that the child is a draft carrying the parent's name in its lineage. It re-checks the parent's trunk count afterwards, so "untouched" means what it says rather than only "still completed". Both fixture modules gain `create_correction` on `completed`, because that is what the server declares — a mock withholding it would describe a batch the API cannot produce. `capabilities.test.ts`'s "names every action the wire has" guard caught the missing constant, which is the job it was written for. One recorder fix: the body-capturing stub in `promote.test.tsx` called `JSON.parse` on every non-GET, and `promote` sends no body — so an empty string threw and failed every test in the file for a reason none of them was about. --- .../information-architecture/SKILL.md | 1 + frontend/app/cycle/cycle.spec.ts | 36 ++++ frontend/app/e2e/_wire.ts | 2 +- frontend/app/src/routes.tsx | 4 + .../ui-core/src/annotator/AnnotationPage.tsx | 29 +++ .../ui-core/src/data/capabilities.test.ts | 11 +- .../ui-core/src/screens/BatchesScreen.tsx | 63 +++++-- .../ui-core/src/screens/GalleryScreen.tsx | 99 +++++++++++ frontend/ui-core/src/screens/promote.test.tsx | 166 +++++++++++++++++- frontend/ui-core/src/testing/wire.fixtures.ts | 2 +- 10 files changed, 396 insertions(+), 17 deletions(-) diff --git a/.agents/skills/frontend/information-architecture/SKILL.md b/.agents/skills/frontend/information-architecture/SKILL.md index 08242ed1..7e2aaeec 100644 --- a/.agents/skills/frontend/information-architecture/SKILL.md +++ b/.agents/skills/frontend/information-architecture/SKILL.md @@ -35,6 +35,7 @@ router, so it can only say what a value resolves to, never change the URL. Rules derived from the 2026-08 audit (§6): +- **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 tab. 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`. - **"Schema history" is not a sibling tab.** Version history lives inside the Schema tab, below the editor and beside the `VersionNavigator` seam. The two overlap on purpose: the navigator is the *reader* (one version, with what it changed), the history is the *ledger* (every version at once). `?tab=versions` remains as a redirect; it does not appear in the tab bar. - **The 4-step checklist is onboarding, not navigation.** It retires itself twice over: when the journey is finished (`hasReleases` makes `done` derivable) and when somebody dismisses it. Dismissal is **per project** and persisted — finishing one project does not teach you the pipeline for the next. It gates nothing and is never the sole path to a screen. diff --git a/frontend/app/cycle/cycle.spec.ts b/frontend/app/cycle/cycle.spec.ts index 4df79598..f9c30d19 100644 --- a/frontend/app/cycle/cycle.spec.ts +++ b/frontend/app/cycle/cycle.spec.ts @@ -408,6 +408,42 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa ); }); + await test.step("correct the completed batch, forward-only", async () => { + /* + * **The end of the forward-only story** (audit G6), against the real kernel. + * + * A completed batch has no exit and none is coming, so the product's answer + * to "this frame is wrong" is a new batch over the same frames recording + * where it came from. Three surfaces had been saying so while nothing could + * create one; this is the control they were pointing at. + * + * Run here rather than against stubs because the two claims worth making are + * about the kernel: that the parent is genuinely untouched, and that the + * child pins the project's *active* schema at its own approval rather than + * inheriting the parent's. + */ + await page.getByTestId("correct-cycle-batch").click(); + await expect(page.getByTestId("correction-dialog")).toBeVisible(); + // The suggested name is the parent's, so the ordinary case costs no typing. + await expect(page.getByTestId("correction-name")).toHaveValue(/cycle-batch/); + await page.getByTestId("correction-submit").click(); + + // It navigates to the correction it just made, and that batch says what it + // corrects. One hop: the child names its parent, and a reader walks the + // chain for the origin. + await expect(page.getByTestId("gallery")).toBeVisible(); + await expect(page.getByTestId("correction-of")).toContainText("Correction of cycle-batch"); + await expect(page.getByTestId("batch-state")).toHaveText("pending approval"); + + // And the parent has not moved — which is the whole point of correcting + // forward instead of reopening. + await openProject(page, "batches"); + await expect(page.getByTestId("state-cycle-batch")).toHaveText("completed"); + await expect(page.getByTestId("promoted-count-cycle-batch")).toHaveText( + /3 of 3 in the dataset/, + ); + }); + await test.step("publish a release", async () => { // **A tab, reached in one press.** It was behind the header's overflow menu, // which is where a destination goes when the navigation has no room for it — diff --git a/frontend/app/e2e/_wire.ts b/frontend/app/e2e/_wire.ts index 2550ee0a..1bb84453 100644 --- a/frontend/app/e2e/_wire.ts +++ b/frontend/app/e2e/_wire.ts @@ -24,7 +24,7 @@ const BATCH_ACTIONS: Record = { draft: ["approve", "edit_membership", "delete"], approved: ["start", "repin", "delete"], in_annotation: ["complete", "repin", "delete"], - completed: ["promote"], + completed: ["promote", "create_correction"], }; const JOB_ACTIONS: Record = { diff --git a/frontend/app/src/routes.tsx b/frontend/app/src/routes.tsx index a08ce9a5..5fe2c812 100644 --- a/frontend/app/src/routes.tsx +++ b/frontend/app/src/routes.tsx @@ -243,6 +243,10 @@ function Gallery(): JSX.Element { // batch is finished, and it had no way to reach the one screen that shows // what finishing it produced — a tab of the project now, not a route. onOpenDataset={() => void navigate(PARENT.dataset(projectId))} + // A correction just cut, or this batch's own parent (audit G6). Same + // route the batch table's rows use — a batch is a batch, whichever screen + // named it. + onOpenBatch={(next) => void navigate(`/projects/${projectId}/batches/${next}`)} /> ); } diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index f1a2943c..7c72ef63 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -1032,6 +1032,35 @@ function Workspace({