diff --git a/docs/ops/image-background-removal-stack.md b/docs/ops/image-background-removal-stack.md index 575033a..3bc8204 100644 --- a/docs/ops/image-background-removal-stack.md +++ b/docs/ops/image-background-removal-stack.md @@ -2,7 +2,7 @@ Setup notes for the Python image-processing stack behind the review queue's background-removal option. -**Status: evaluated, not adopted.** The engine choice is still open — see the "Alternatives ruled out" section. Everything below was run and measured on 2026-09-02 against `danielgatis/rembg:latest`, on a Windows dev box under Docker Desktop. **The NAS is a different machine and will be slower**; treat these as an upper bound on capability, not a promise. +**Status: adopted (#281).** The engine choice is settled — see "The one thing that must not be got wrong" below. Everything here was run and measured on 2026-09-02 against `danielgatis/rembg:latest`, on a Windows dev box under Docker Desktop. **The NAS is a different machine and will be slower**; treat these as an upper bound on capability, not a promise. ## Why a sidecar and not the host's Python @@ -88,8 +88,17 @@ curl -s -F "file=@any.jpg" -F "model=u2net" -o /dev/null http://localhost:7000/a **A hosted API** (remove.bg, Photoroom) — clear terms, no disk cost, best quality. Roughly $0.20 an image, another credential, another external dependency in the pipeline, and would want the budget ceiling that #227 gave submissions. -## If this is adopted +## How the application uses it -The application would `POST` to the sidecar rather than doing any inference itself, which keeps every Python and ONNX dependency out of the Node image. Given 1–2 s warm, a synchronous request from the review queue is reasonable — unlike drafting, which was deliberately moved off the request path because a stranger can trigger it and must never wait. +`backend/src/intake/rembgClient.ts` is the only thing that talks to the sidecar. It posts to `/api/remove` and **always sends `model=u2net`**; a unit test asserts that parameter is present, because nothing about the returned image would reveal its absence. -The four design decisions already taken are independent of the engine: the cut-out replaces the item's image while the original is kept and restorable, the result is a transparent PNG, and the control sits on each photo in the review queue. +`REMBG_URL` points at it — `http://rembg-syn:7000` on the NAS, where the container publishes `32700:7000`. The variable is optional in both compose files: unset means the submission page shows no checkbox, the review queue shows no control, and the drafting worker skips the step. An environment without a sidecar is a working environment. + +Two entry points, one module (`backend/src/intake/backgroundRemoval.ts`): + +- **The drafting worker**, honouring the checkbox on `/submit/:token`, which is ticked by default. The submitter's tick is recorded and acted on later, so nobody waits on a model run and an unreachable sidecar cannot fail an upload. +- **The review queue**, per photo, synchronously — 1.1–2.3 s warm is a wait an admin who just clicked a button can absorb. + +Because removal follows drafting in the worker, an environment with no `ANTHROPIC_API_KEY` drafts nothing and so cuts out nothing. The per-photo control in the review queue is the way to do it by hand there. + +The original file is never destroyed. `item_images.original_image_path` records where it went, and **Restore original** swaps it back. Nothing in the feature deletes a file or a row. diff --git a/frontend/src/admin/DraftQueue.tsx b/frontend/src/admin/DraftQueue.tsx index 9d271ed..96f729e 100644 --- a/frontend/src/admin/DraftQueue.tsx +++ b/frontend/src/admin/DraftQueue.tsx @@ -10,7 +10,15 @@ import Empty from 'antd/es/empty'; import Alert from 'antd/es/alert'; import Modal from 'antd/es/modal'; import message from 'antd/es/message'; -import { Draft, PriceSource, actOnDraft, fetchDrafts, publishDraft } from './draftsApi'; +import { + Draft, + DraftImage, + PriceSource, + actOnDraft, + fetchDrafts, + publishDraft, + setImageBackground +} from './draftsApi'; const { TextArea } = Input; @@ -25,6 +33,54 @@ function priceLabel(source: PriceSource): string { return 'default price — nobody chose this'; } +/** + * One photo, with the control that cuts it out or puts it back. + * + * Per photo rather than per item because that is how a poor result is undone: + * background removal produces the occasional bad cut on an unusual object, and + * the answer is to restore that one photograph, not to unpick the submission. + * + * The label is the state. `original_image_path` is the only thing consulted, + * so there is no second flag that could disagree with what the button does. + */ +function DraftPhoto({ + image, + itemId, + enabled, + onChanged +}: Readonly<{ + image: DraftImage; + itemId: number; + enabled: boolean; + onChanged: () => void; +}>) { + const [busy, setBusy] = useState(false); + const cutOut = image.original_image_path !== null; + + const act = async () => { + setBusy(true); + try { + await setImageBackground(itemId, image.id, cutOut ? 'restore-original' : 'remove-background'); + onChanged(); + } catch (err) { + message.error(err instanceof Error ? err.message : 'that did not work'); + } finally { + setBusy(false); + } + }; + + return ( + + + {enabled && ( + + )} + + ); +} + /** * One submission, with everything needed to judge it. * @@ -33,7 +89,11 @@ function priceLabel(source: PriceSource): string { * so — and 80.00 is a plausible price rather than an obvious sentinel, which is * exactly why it has to be called out rather than left to be noticed. */ -function DraftCard({ draft, onChanged }: Readonly<{ draft: Draft; onChanged: () => void }>) { +function DraftCard({ + draft, + backgroundRemoval, + onChanged +}: Readonly<{ draft: Draft; backgroundRemoval: boolean; onChanged: () => void }>) { const [name, setName] = useState(draft.ai_name ?? draft.item_name); const [description, setDescription] = useState( draft.ai_description ?? draft.item_description ?? '' @@ -82,13 +142,14 @@ function DraftCard({ draft, onChanged }: Readonly<{ draft: Draft; onChanged: () {draft.ai_error && } - + {draft.images.map((image) => ( - ))} @@ -148,12 +209,15 @@ function DraftCard({ draft, onChanged }: Readonly<{ draft: Draft; onChanged: () export default function DraftQueue() { const [drafts, setDrafts] = useState([]); + const [backgroundRemoval, setBackgroundRemoval] = useState(false); const [state, setState] = useState(undefined); const [error, setError] = useState(null); const load = useCallback(async () => { try { - setDrafts(await fetchDrafts(state)); + const payload = await fetchDrafts(state); + setDrafts(payload.drafts); + setBackgroundRemoval(payload.backgroundRemoval); setError(null); } catch { setError('Could not load the review queue.'); @@ -188,7 +252,12 @@ export default function DraftQueue() { {error && } {!error && drafts.length === 0 && } {drafts.map((draft) => ( - void load()} /> + void load()} + /> ))} ); diff --git a/frontend/src/admin/draftsApi.ts b/frontend/src/admin/draftsApi.ts index b8b6e49..aaa3555 100644 --- a/frontend/src/admin/draftsApi.ts +++ b/frontend/src/admin/draftsApi.ts @@ -3,6 +3,12 @@ export type PriceSource = 'default' | 'ai' | 'admin'; export interface DraftImage { id: number; image_path: string; + /** + * Where this photo came from, once it has been cut out. Null means it never + * was — which is also the answer to whether Restore has anything to do, so + * there is no second flag that could disagree with it. + */ + original_image_path: string | null; } export interface Draft { @@ -31,11 +37,21 @@ async function send(path: string, init?: RequestInit): Promise { }); } -export async function fetchDrafts(state?: string): Promise { +export interface DraftQueueResponse { + drafts: Draft[]; + /** + * Whether a background-removal sidecar is configured. False hides the + * control rather than showing one that would answer 502 — an environment + * without a sidecar is a working environment. + */ + backgroundRemoval: boolean; +} + +export async function fetchDrafts(state?: string): Promise { const query = state ? `?state=${encodeURIComponent(state)}` : ''; const res = await send(query); if (!res.ok) throw new Error('could not load the review queue'); - return (await res.json()).drafts; + return res.json(); } export interface PublishInput { @@ -71,3 +87,29 @@ export async function actOnDraft( const res = await send(`/${itemId}/${action}`, { method: 'POST' }); if (!res.ok) throw new Error(`could not ${action} this draft`); } + +/** + * Cut one photo out, or put its original back. + * + * The server's message is preferred over a generic one for the same reason + * publishDraft prefers it: a 502 here says the removal service did not answer + * and the photo is unchanged, which is the difference between "try again" and + * "something is wrong with this item". + */ +export async function setImageBackground( + itemId: number, + imageId: number, + action: 'remove-background' | 'restore-original' +): Promise { + const res = await send(`/${itemId}/images/${imageId}/${action}`, { method: 'POST' }); + if (res.ok) return; + + let message = 'could not change this photo'; + try { + message = (await res.json()).error ?? message; + } catch { + // A non-JSON body is a proxy or gateway error rather than the app + // refusing. The generic message is the honest thing to show. + } + throw new Error(message); +} diff --git a/frontend/tests/e2e/admin-draft-queue.spec.ts b/frontend/tests/e2e/admin-draft-queue.spec.ts index f952f83..5fe3720 100644 --- a/frontend/tests/e2e/admin-draft-queue.spec.ts +++ b/frontend/tests/e2e/admin-draft-queue.spec.ts @@ -98,4 +98,23 @@ test.describe('The review queue', () => { await dialog.getByRole('button', { name: 'Cancel', exact: true }).click(); await expect(card.getByText(/nobody chose this/)).toBeVisible(); }); + + // The control that makes a poor cut survivable. Its label is its state: + // "Remove background" until an original has been recorded, "Restore + // original" afterwards, read from one field rather than two that could + // disagree. + // + // Only the label is asserted, not a click. Pressing it would need a sidecar, + // and a test that depends on a service taking forty seconds to start is + // broken by construction — the swap itself is covered in the integration + // suite against a stub. + test('offers to remove the background on each photo', async ({ page, admin }) => { + const note = `Cutout ${RUN}`; + await submitAnItem(page, note); + + await admin.open('Review queue'); + + const card = page.locator('.ant-card').filter({ hasText: note }); + await expect(card.getByRole('button', { name: 'Remove background' })).toBeVisible(); + }); });