From 75fbfd2293294d14b81e34ea15ad342b69fc452a Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 06:52:16 -0700 Subject: [PATCH 1/2] feat(ui): delete a pre-processing recipe from the recipe list --- docs/content/preprocessing.md | 2 +- docs/content/ui.md | 5 +- frontend/app/cycle/cycle.spec.ts | 44 ++++++--- frontend/ui-core/src/patterns/RecipeList.tsx | 31 +++++-- .../ui-core/src/screens/PreprocessingTab.tsx | 90 ++++++++++++++++++- .../src/screens/preprocessing.test.tsx | 85 ++++++++++++++++++ 6 files changed, 236 insertions(+), 21 deletions(-) diff --git a/docs/content/preprocessing.md b/docs/content/preprocessing.md index d8804985..e19920c8 100644 --- a/docs/content/preprocessing.md +++ b/docs/content/preprocessing.md @@ -181,7 +181,7 @@ scaled to match, and the response is never cached: the spec is the request's own | | | | --- | --- | -| UI | The Dataset section's **Pre-processing** view: the project's recipes as a list, an editor of four steps (target model, resize, augmentation, preview), and *Save recipe*; the Export dialog's **Pre-processing recipe** control chooses one by name, `None` by default. See [ui.md](ui.md#the-dataset-its-releases-and-getting-the-data-out). | +| UI | The Dataset section's **Pre-processing** view: the project's recipes as a list, an editor of four steps (target model, resize, augmentation, preview), *Save recipe*, and a delete control on each row that asks first; the Export dialog's **Pre-processing recipe** control chooses one by name, `None` by default. See [ui.md](ui.md#the-dataset-its-releases-and-getting-the-data-out). | | REST | `POST`/`GET /projects/{id}/preprocessing-recipes`, `GET`/`PUT`/`DELETE /projects/{id}/preprocessing-recipes/{name}`, `POST /projects/{id}/preprocessing-preview`; `recipe=` on `POST /releases/{id}/export` and `GET /releases/{id}/export-compatibility`. The job carries the recipe as a snapshot. | | CLI | `visionset recipe create NAME -p P --spec FILE` or `--resize letterbox:640x640 --augment hflip,brightness_contrast --variants 2 --target yolo11`; `recipe list`, `show`, `update`, `delete`; `export --recipe NAME`. See [cli.md](cli.md#visionset-release-and-visionset-export). | | MCP | `create_preprocessing_recipe`, `list_preprocessing_recipes`, `delete_preprocessing_recipe` (only with `--allow-destructive`); `recipe` on `export_release` and `check_export`. See [mcp.md](mcp.md#datasets-releases-and-export). | diff --git a/docs/content/ui.md b/docs/content/ui.md index bc5c15b6..e4f5a5bb 100644 --- a/docs/content/ui.md +++ b/docs/content/ui.md @@ -283,7 +283,10 @@ not a JPEG, PNG or WebP — is prose in the cell. Save is a create for a new draft and a whole-replace `PUT` at the recipe's current name for an open one; Discard puts the stored spec back; `PREPROCESSING_RECIPE_NAME_TAKEN` and every -other refusal render through the vocabulary. The Export dialog's second control, +other refusal render through the vocabulary. Each row carries a delete control beside it +that asks first — the recipe alone goes, and an export already run keeps its own copy — and +closes the editor when the recipe it held is the one deleted; a refusal such as +`PREPROCESSING_RECIPE_NOT_FOUND` is prose in the dialog. The Export dialog's second control, **Pre-processing recipe**, lists the same recipes with the same one-line summary under *None*, and sends the chosen name as `recipe=`; a project with none says so in a line rather than offering a picker with one row. The two 409s an export can answer — diff --git a/frontend/app/cycle/cycle.spec.ts b/frontend/app/cycle/cycle.spec.ts index dc90f7ad..460723c6 100644 --- a/frontend/app/cycle/cycle.spec.ts +++ b/frontend/app/cycle/cycle.spec.ts @@ -1619,6 +1619,33 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa await expectRecipeArchive(download); }); + await test.step("delete the recipe, and watch the list and the count answer", async () => { + /* + * The last verb a recipe has, after the export that needed it has run: + * the row's control, the confirmation, the real `DELETE`, and the view + * back to its invitation with the tab no longer counting. The editor was + * holding this recipe, so its closing is part of what is asserted. + */ + // The export dialog is still up, and deliberately: it holds the outcome so + // the badge can announce it once the poll has stopped, which means it closes + // the way every other dialog here does rather than on its own. So this step + // starts by doing what the person who has just taken the download does. The + // walk dismisses a dialog this way once already, when the batch is deleted. + await page.keyboard.press("Escape"); + await expect(page.getByTestId("export-dialog")).toHaveCount(0); + + await page.getByTestId("dataset-tab-preprocessing").click(); + await expect(page.getByTestId("recipe-editor")).toBeVisible(); + await page.getByTestId(`recipe-delete-${RECIPE}`).click(); + await expect(page.getByTestId("delete-recipe-dialog")).toContainText(`Delete ${RECIPE}?`); + await page.getByTestId("delete-recipe-submit").click(); + + await expect(page.getByTestId("delete-recipe-dialog")).toHaveCount(0); + await expect(page.getByTestId("recipes-empty")).toBeVisible(); + await expect(page.getByTestId("recipe-editor")).toHaveCount(0); + await expect(page.getByTestId("dataset-tab-preprocessing")).toContainText("0"); + }); + await test.step("edit the connection, and watch the row answer for it", async () => { /* * A stub cannot referee this body, because it is written by whoever wrote @@ -1633,14 +1660,6 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa * transcription. Last in the walk and nothing is put back, since no later * step reads this connection. */ - // The export dialog is still up, and deliberately: it holds the outcome so - // the badge can announce it once the poll has stopped, which means it closes - // the way every other dialog here does rather than on its own. So this step - // starts by doing what the person who has just taken the download does. The - // walk dismisses a dialog this way once already, in the delete step. - await page.keyboard.press("Escape"); - await expect(page.getByTestId("export-dialog")).toHaveCount(0); - await page.getByTestId("rail-models").click(); await expect(page.getByTestId("models-screen")).toBeVisible(); // Two locators because the row's id is its name and the rename moves it. @@ -1700,7 +1719,7 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa /* * **The aborted API calls, pinned rather than filtered away.** * - * Both entries answer `204 No Content`, and Chromium reports every such + * Every entry answers `204 No Content`, and Chromium reports every such * request as `net::ERR_ABORTED` — there is no body for the renderer to read, * so the network stack tears the stream down and files it as cancelled. * Measured, not assumed, for the first one: the deletion is committed (the @@ -1713,9 +1732,11 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa * sends; the step that calls it asserts the batch is gone from the table and * the trunk did not move, which is what "committed" means there. The * annotation delete had no coverage before this walk at all, because nothing - * in the browser had ever deleted an annotation. + * in the browser had ever deleted an annotation. The recipe delete is the + * third, committed the same way: the view was back at its invitation and + * the tab had stopped counting before the walk moved on. * - * Asserted as an exact list, in walk order: a *third* aborted call, or one on + * Asserted as an exact list, in walk order: a *fourth* aborted call, or one on * another route, is the shape of a request the app really did abandon, and * that is worth failing on. * @@ -1727,6 +1748,7 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa expect(abortedApiCalls).toEqual([ expect.stringMatching(/^DELETE .*\/batches\/[0-9a-f-]+$/), expect.stringMatching(/^DELETE .*\/annotations$/), + expect.stringMatching(/^DELETE .*\/preprocessing-recipes\/[^/]+$/), ]); /* * **The refused API calls, pinned rather than merely tolerated.** diff --git a/frontend/ui-core/src/patterns/RecipeList.tsx b/frontend/ui-core/src/patterns/RecipeList.tsx index d0792d5e..3d83fa95 100644 --- a/frontend/ui-core/src/patterns/RecipeList.tsx +++ b/frontend/ui-core/src/patterns/RecipeList.tsx @@ -8,10 +8,12 @@ * summary and, as a `quiet` chip, the target its hints were read from: a fact * beside other facts, never a state. * - * Data-only. The screen decides what is selected and what `New` does. + * Data-only. The screen decides what is selected, what `New` does, and what + * deleting asks first. The delete control sits beside the row rather than + * inside it: the row is itself a button, and a button cannot hold one. */ -import { Plus } from "lucide-react"; +import { Plus, Trash2 } from "lucide-react"; import type { JSX } from "react"; import { cn } from "../lib/cn"; @@ -30,6 +32,7 @@ export interface RecipeListProps { readonly selected: string | null; readonly onSelect: (name: string) => void; readonly onNew: () => void; + readonly onDelete: (name: string) => void; /** The label a target's `name` is shown as; an unknown target shows its name. */ readonly labelFor: (target: string) => string; readonly className?: string; @@ -40,6 +43,7 @@ export function RecipeList({ selected, onSelect, onNew, + onDelete, labelFor, className, }: RecipeListProps): JSX.Element { @@ -56,7 +60,13 @@ export function RecipeList({ {recipes.map((recipe) => { const chosen = recipe.name === selected; return ( -
  • +
  • +
    • ); })} diff --git a/frontend/ui-core/src/screens/PreprocessingTab.tsx b/frontend/ui-core/src/screens/PreprocessingTab.tsx index 65d30825..900498da 100644 --- a/frontend/ui-core/src/screens/PreprocessingTab.tsx +++ b/frontend/ui-core/src/screens/PreprocessingTab.tsx @@ -8,7 +8,8 @@ * unconditional, so this screen gates nothing. What it holds is a draft — the * stored recipe as typed fields — and `dirty` is the draft disagreeing with the * spec it opened from. Save is a create for a new draft and a whole-replace - * `PUT` for an open one; Discard puts the stored spec back. + * `PUT` for an open one; Discard puts the stored spec back; Delete asks first, + * and closes the editor when the recipe it held is the one that went. * * ## The preview is the export's own path * @@ -37,9 +38,18 @@ import { RecipeEditor } from "../patterns/RecipeEditor"; import { RecipeList } from "../patterns/RecipeList"; import { StaticAnnotationOverlay } from "../patterns/StaticAnnotationOverlay"; import { Button } from "../primitives/Button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogTitle, +} from "../primitives/Dialog"; +import { FieldError } from "../primitives/Input"; import { useActiveSchema, useCreatePreprocessingRecipe, + useDeletePreprocessingRecipe, useExportTargets, usePreprocessingPreview, usePreprocessingRecipes, @@ -83,6 +93,7 @@ export function PreprocessingTab({ projectId, datasetId }: PreprocessingTabProps const create = useCreatePreprocessingRecipe(projectId); const update = useUpdatePreprocessingRecipe(projectId); const [editing, setEditing] = useState(null); + const [deleting, setDeleting] = useState(null); const items = recipes.data?.items ?? []; const catalog = targets.data?.items ?? []; @@ -157,8 +168,22 @@ export function PreprocessingTab({ projectId, datasetId }: PreprocessingTabProps } }} onNew={startNew} + onDelete={setDeleting} labelFor={labelFor} /> + setDeleting(null)} + onDeleted={(name) => { + // The list still holds the deleted row until its refetch lands, so + // the next recipe is chosen here rather than left to the effect, + // which would reopen the one that has just gone. + if (editing?.name !== name) return; + const next = items.find((one) => one.name !== name); + setEditing(next === undefined ? null : open(next)); + }} + /> {editing !== null && ( void; + readonly onDeleted: (name: string) => void; +}): JSX.Element { + const remove = useDeletePreprocessingRecipe(projectId); + + return ( + { + if (next) return; + remove.reset(); + onClose(); + }} + > + + Delete {name}? + + The recipe is removed from this project and can no longer be chosen at export. Releases + already exported through it keep their files: an export carries its own copy of the + recipe it ran. + + {remove.isError && ( + {refusalProse(remove.error)} + )} + + + + + + + ); +} + function Editor({ projectId, datasetId, diff --git a/frontend/ui-core/src/screens/preprocessing.test.tsx b/frontend/ui-core/src/screens/preprocessing.test.tsx index 033f055c..f3016ade 100644 --- a/frontend/ui-core/src/screens/preprocessing.test.tsx +++ b/frontend/ui-core/src/screens/preprocessing.test.tsx @@ -528,6 +528,91 @@ describe("the editor", () => { }); }); +describe("deleting a recipe", () => { + it("asks first, sends DELETE at the recipe's name, and the row leaves the list", async () => { + const stored = [recipeRow("yolo-640", LETTERBOX), recipeRow("wide-720", { ...LETTERBOX, target: "wide" })]; + handlers.push((request) => { + if (request.method !== "GET" || !pathOf(request).endsWith("/preprocessing-recipes")) return undefined; + const items = sent.some((r) => r.method === "DELETE") ? stored.slice(1) : stored; + return { status: 200, body: { items, total: items.length } }; + }); + baseline(); + on("DELETE", /\/preprocessing-recipes\/yolo-640$/, { status: 204 }); + render(mount()); + await screen.findByTestId("recipe-editor"); + expect(screen.getByTestId("recipe-yolo-640").getAttribute("aria-current")).toBe("true"); + + await userEvent.click(screen.getByTestId("recipe-delete-yolo-640")); + const dialog = await screen.findByTestId("delete-recipe-dialog"); + expect(dialog.textContent).toContain("Delete yolo-640?"); + expect(sent.some((r) => r.method === "DELETE")).toBe(false); + await userEvent.click(within(dialog).getByTestId("delete-recipe-submit")); + + const remove = () => sent.find((r) => r.method === "DELETE"); + await waitFor(() => expect(remove()).not.toBeUndefined()); + expect(pathOf(remove() as Request)).toMatch(/\/preprocessing-recipes\/yolo-640$/); + await waitFor(() => expect(screen.queryByTestId("delete-recipe-dialog")).toBeNull()); + await waitFor(() => expect(screen.queryByTestId("recipe-yolo-640")).toBeNull()); + // The editor held the deleted one, so it moves to the recipe that is left. + expect(screen.getByTestId("recipe-wide-720").getAttribute("aria-current")).toBe("true"); + expect(screen.getByTestId("recipe-name")).toHaveProperty("value", "wide-720"); + }); + + it("returns to the invitation when the last recipe goes", async () => { + const stored = [recipeRow("yolo-640", LETTERBOX)]; + handlers.push((request) => { + if (request.method !== "GET" || !pathOf(request).endsWith("/preprocessing-recipes")) return undefined; + const items = sent.some((r) => r.method === "DELETE") ? [] : stored; + return { status: 200, body: { items, total: items.length } }; + }); + baseline(); + on("DELETE", /\/preprocessing-recipes\/yolo-640$/, { status: 204 }); + render(mount()); + await screen.findByTestId("recipe-editor"); + + await userEvent.click(screen.getByTestId("recipe-delete-yolo-640")); + await userEvent.click(await screen.findByTestId("delete-recipe-submit")); + + await screen.findByTestId("recipes-empty"); + expect(screen.queryByTestId("recipe-editor")).toBeNull(); + }); + + it("renders a refused delete as prose in the dialog, never as its code", async () => { + baseline([recipeRow("yolo-640", LETTERBOX)]); + on("DELETE", /\/preprocessing-recipes\/yolo-640$/, { + status: 404, + body: { code: "PREPROCESSING_RECIPE_NOT_FOUND", message: "project 1111 has no recipe named 'yolo-640'" }, + }); + render(mount()); + await screen.findByTestId("recipe-editor"); + + await userEvent.click(screen.getByTestId("recipe-delete-yolo-640")); + await userEvent.click(await screen.findByTestId("delete-recipe-submit")); + + const said = (await screen.findByTestId("delete-recipe-error")).textContent ?? ""; + expect(said).toContain("no longer on record"); + expect(said).not.toContain("PREPROCESSING_RECIPE_NOT_FOUND"); + expect(said).not.toContain("1111"); + expect(document.body.textContent).not.toContain("PREPROCESSING_RECIPE_NOT_FOUND"); + // The dialog stays up holding the refusal, and the editor still holds the recipe. + expect(screen.getByTestId("delete-recipe-dialog")).toBeTruthy(); + expect(screen.getByTestId("recipe-editor")).toBeTruthy(); + }); + + it("can be cancelled without a request", async () => { + baseline([recipeRow("yolo-640", LETTERBOX)]); + render(mount()); + await screen.findByTestId("recipe-editor"); + + await userEvent.click(screen.getByTestId("recipe-delete-yolo-640")); + await userEvent.click(await screen.findByTestId("delete-recipe-cancel")); + + await waitFor(() => expect(screen.queryByTestId("delete-recipe-dialog")).toBeNull()); + expect(sent.some((r) => r.method === "DELETE")).toBe(false); + expect(screen.getByTestId("recipe-yolo-640").getAttribute("aria-current")).toBe("true"); + }); +}); + describe("the preview", () => { it("samples the project's first three assets when no release has a split, and renders each stage", async () => { baseline([recipeRow("yolo-640", LETTERBOX)]); From 1508d2a747159abd72f6c9302111635732519f17 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 06:56:19 -0700 Subject: [PATCH 2/2] fix(ui): the recipe preview grid spends its width on the three images --- frontend/ui-core/src/screens/PreprocessingTab.tsx | 9 +++------ frontend/ui-core/src/screens/preprocessing.test.tsx | 5 ++++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frontend/ui-core/src/screens/PreprocessingTab.tsx b/frontend/ui-core/src/screens/PreprocessingTab.tsx index 900498da..a95f1c61 100644 --- a/frontend/ui-core/src/screens/PreprocessingTab.tsx +++ b/frontend/ui-core/src/screens/PreprocessingTab.tsx @@ -436,8 +436,7 @@ function PreviewGrid({ return (
    -
    - +
    Original After resize After augmentation @@ -445,12 +444,10 @@ function PreviewGrid({ {samples.map((assetId, index) => (
    - - {assetId.slice(0, 8)} - { render(mount()); const grid = await screen.findByTestId("preview-grid"); - expect(within(grid).getAllByTestId(/^preview-row-/)).toHaveLength(3); + const rows = within(grid).getAllByTestId(/^preview-row-/); + expect(rows).toHaveLength(3); + // Three cells and nothing else: the row spends its width on the images. + for (const row of rows) expect(row.children).toHaveLength(3); expect(screen.getByTestId("preview-aside").textContent).toBe("3 sample assets · seeded"); await waitFor(() =>