Feature/281 background removal plan #284

Merged
bermudalamb merged 17 commits from feature/281-background-removal-plan into main 2026-09-03 14:09:52 -05:00
4 changed files with 154 additions and 15 deletions
Showing only changes of commit 09686d9f94 - Show all commits
+13 -4
View File
@@ -2,7 +2,7 @@
Setup notes for the Python image-processing stack behind the review queue's background-removal option. 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 ## 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. **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 12 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.12.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.
+78 -9
View File
@@ -10,7 +10,15 @@ import Empty from 'antd/es/empty';
import Alert from 'antd/es/alert'; import Alert from 'antd/es/alert';
import Modal from 'antd/es/modal'; import Modal from 'antd/es/modal';
import message from 'antd/es/message'; 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; const { TextArea } = Input;
@@ -25,6 +33,54 @@ function priceLabel(source: PriceSource): string {
return 'default price — nobody chose this'; 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 (
<Space direction="vertical" size={4} align="center">
<img src={image.image_path} alt="" style={{ width: 120, height: 120, objectFit: 'cover' }} />
{enabled && (
<Button size="small" loading={busy} onClick={() => void act()}>
{cutOut ? 'Restore original' : 'Remove background'}
</Button>
)}
</Space>
);
}
/** /**
* One submission, with everything needed to judge it. * 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 * 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. * 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 [name, setName] = useState(draft.ai_name ?? draft.item_name);
const [description, setDescription] = useState( const [description, setDescription] = useState(
draft.ai_description ?? draft.item_description ?? '' draft.ai_description ?? draft.item_description ?? ''
@@ -82,13 +142,14 @@ function DraftCard({ draft, onChanged }: Readonly<{ draft: Draft; onChanged: ()
<Space direction="vertical" style={{ width: '100%' }} size="middle"> <Space direction="vertical" style={{ width: '100%' }} size="middle">
{draft.ai_error && <Alert type="warning" message={`Drafting failed: ${draft.ai_error}`} />} {draft.ai_error && <Alert type="warning" message={`Drafting failed: ${draft.ai_error}`} />}
<Space wrap> <Space wrap align="start">
{draft.images.map((image) => ( {draft.images.map((image) => (
<img <DraftPhoto
key={image.id} key={image.id}
src={image.image_path} image={image}
alt="" itemId={draft.item_id}
style={{ width: 120, height: 120, objectFit: 'cover' }} enabled={backgroundRemoval}
onChanged={onChanged}
/> />
))} ))}
</Space> </Space>
@@ -148,12 +209,15 @@ function DraftCard({ draft, onChanged }: Readonly<{ draft: Draft; onChanged: ()
export default function DraftQueue() { export default function DraftQueue() {
const [drafts, setDrafts] = useState<Draft[]>([]); const [drafts, setDrafts] = useState<Draft[]>([]);
const [backgroundRemoval, setBackgroundRemoval] = useState(false);
const [state, setState] = useState<string | undefined>(undefined); const [state, setState] = useState<string | undefined>(undefined);
const [error, setError] = useState<string | null>(null); const [error, setError] = useState<string | null>(null);
const load = useCallback(async () => { const load = useCallback(async () => {
try { try {
setDrafts(await fetchDrafts(state)); const payload = await fetchDrafts(state);
setDrafts(payload.drafts);
setBackgroundRemoval(payload.backgroundRemoval);
setError(null); setError(null);
} catch { } catch {
setError('Could not load the review queue.'); setError('Could not load the review queue.');
@@ -188,7 +252,12 @@ export default function DraftQueue() {
{error && <Alert type="error" message={error} />} {error && <Alert type="error" message={error} />}
{!error && drafts.length === 0 && <Empty description="Nothing waiting for review" />} {!error && drafts.length === 0 && <Empty description="Nothing waiting for review" />}
{drafts.map((draft) => ( {drafts.map((draft) => (
<DraftCard key={draft.item_id} draft={draft} onChanged={() => void load()} /> <DraftCard
key={draft.item_id}
draft={draft}
backgroundRemoval={backgroundRemoval}
onChanged={() => void load()}
/>
))} ))}
</div> </div>
); );
+44 -2
View File
@@ -3,6 +3,12 @@ export type PriceSource = 'default' | 'ai' | 'admin';
export interface DraftImage { export interface DraftImage {
id: number; id: number;
image_path: string; 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 { export interface Draft {
@@ -31,11 +37,21 @@ async function send(path: string, init?: RequestInit): Promise<Response> {
}); });
} }
export async function fetchDrafts(state?: string): Promise<Draft[]> { 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<DraftQueueResponse> {
const query = state ? `?state=${encodeURIComponent(state)}` : ''; const query = state ? `?state=${encodeURIComponent(state)}` : '';
const res = await send(query); const res = await send(query);
if (!res.ok) throw new Error('could not load the review queue'); if (!res.ok) throw new Error('could not load the review queue');
return (await res.json()).drafts; return res.json();
} }
export interface PublishInput { export interface PublishInput {
@@ -71,3 +87,29 @@ export async function actOnDraft(
const res = await send(`/${itemId}/${action}`, { method: 'POST' }); const res = await send(`/${itemId}/${action}`, { method: 'POST' });
if (!res.ok) throw new Error(`could not ${action} this draft`); 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<void> {
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);
}
@@ -98,4 +98,23 @@ test.describe('The review queue', () => {
await dialog.getByRole('button', { name: 'Cancel', exact: true }).click(); await dialog.getByRole('button', { name: 'Cancel', exact: true }).click();
await expect(card.getByText(/nobody chose this/)).toBeVisible(); 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();
});
}); });