feat(intake): cut out backgrounds in the worker, never in the upload (#281)
Wires removeBackgroundsForItem into draftQueued, gated on the submitter's remove_background intent recorded on item_drafts. The step runs after the draft is committed and catches for itself, so an unreachable or erroring sidecar never turns a draft that was written correctly into a failed one — the photo simply keeps its original, and the admin's per-photo control in the review queue is still there to do it by hand. It is awaited, unlike the notification below it, so a sweep that has returned has finished its work; nothing on the request path waits on it. Adds backend/tests/integration/draftingBackgroundRemoval.integration.test.ts as a new file rather than extending drafting.integration.test.ts, because that suite has never produced a successful draft and therefore has no draftListing mock — adding one there would be file-wide and would change what its existing tests exercise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@ import { getAnthropicClient } from './anthropicClient';
|
||||
import { draftListing } from './draftListing';
|
||||
import { applyDraft } from './applyDraft';
|
||||
import { notifyDraftReady } from './notifyDraft';
|
||||
import { removeBackgroundsForItem } from './backgroundRemoval';
|
||||
|
||||
/**
|
||||
* Turns queued submissions into drafts.
|
||||
@@ -19,6 +20,13 @@ import { notifyDraftReady } from './notifyDraft';
|
||||
* The photos are often the only copy of an item no longer in the sender's
|
||||
* hands, so every failure below leaves the row and its images intact and merely
|
||||
* undrafted. Nothing in this file deletes anything.
|
||||
*
|
||||
* Background removal (#281) follows drafting rather than running on its own
|
||||
* pass. That couples the two: an environment with no ANTHROPIC_API_KEY drafts
|
||||
* nothing, so it cuts out nothing either. That is the intended trade — a
|
||||
* separate pass would re-attempt an unreachable sidecar on every sweep for a
|
||||
* row that is going to sit at 'queued' indefinitely — and the admin's per-photo
|
||||
* control in the review queue is the way to do it by hand meanwhile.
|
||||
*/
|
||||
|
||||
/** Three tries, then it waits for a person rather than burning money on a loop. */
|
||||
@@ -32,6 +40,7 @@ type Photo = { mediaType: string; base64: string };
|
||||
interface QueuedRow {
|
||||
item_id: number;
|
||||
submitter_note: string | null;
|
||||
remove_background: boolean;
|
||||
}
|
||||
|
||||
export interface SweepResult {
|
||||
@@ -119,7 +128,7 @@ async function draftOne(
|
||||
|
||||
export async function draftQueued(limit = DEFAULT_BATCH): Promise<SweepResult> {
|
||||
const { rows } = await pool.query<QueuedRow>(
|
||||
`SELECT item_id, submitter_note FROM item_drafts
|
||||
`SELECT item_id, submitter_note, remove_background FROM item_drafts
|
||||
WHERE state = 'queued' AND attempts < $2
|
||||
ORDER BY created_at
|
||||
LIMIT $1`,
|
||||
@@ -146,6 +155,22 @@ export async function draftQueued(limit = DEFAULT_BATCH): Promise<SweepResult> {
|
||||
await draftOne(client, row.item_id, row.submitter_note, photos);
|
||||
drafted++;
|
||||
|
||||
// Deliberately after the draft is committed, and catching for itself.
|
||||
//
|
||||
// This is the sender's tick from the submission page, honoured here so
|
||||
// they never waited for it — and a failure must not mark a draft that was
|
||||
// written correctly as failed. The photo keeps its original in that case,
|
||||
// and the admin's per-photo control is still there to do it by hand.
|
||||
//
|
||||
// Awaited, unlike the notification below, so a sweep that has returned
|
||||
// has finished its work. Nothing is waiting on this: the worker is off
|
||||
// the request path, which is the whole reason drafting lives here.
|
||||
if (row.remove_background) {
|
||||
await removeBackgroundsForItem(row.item_id).catch((err) =>
|
||||
console.error(`[drafting] background removal for item ${row.item_id}:`, err)
|
||||
);
|
||||
}
|
||||
|
||||
// Fire and forget, and deliberately after the draft is committed. A mail
|
||||
// failure must never mark a draft that was written correctly as failed —
|
||||
// the queue is what the admin actually works from, and the email is a
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
import http from 'http';
|
||||
import { AddressInfo } from 'net';
|
||||
import { promises as fsp } from 'fs';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import { pool } from '../../src/db';
|
||||
import { resetDb, closeDb } from './setup/testDb';
|
||||
import { draftQueued } from '../../src/intake/draftingWorker';
|
||||
import { resetAnthropicClient } from '../../src/intake/anthropicClient';
|
||||
import { draftListing } from '../../src/intake/draftListing';
|
||||
|
||||
/**
|
||||
* The worker's background-removal step (#281).
|
||||
*
|
||||
* The model is mocked rather than reached. What is under test is what the
|
||||
* worker does *after* a draft is written — which of the two paths it takes,
|
||||
* and what survives when the sidecar does not answer — and none of that
|
||||
* depends on what the model said.
|
||||
*/
|
||||
jest.mock('../../src/intake/draftListing');
|
||||
|
||||
const draftListingMock = draftListing as jest.MockedFunction<typeof draftListing>;
|
||||
|
||||
const JPEG_BYTES = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0, 0, 0, 0, 0, 0, 0, 0]);
|
||||
const PNG_BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0]);
|
||||
|
||||
let uploads = '';
|
||||
let stub: http.Server | null = null;
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetDb();
|
||||
|
||||
draftListingMock.mockResolvedValue({
|
||||
draft: {
|
||||
name: 'Blue stoneware vase',
|
||||
description: 'Hand-thrown, chipped base.',
|
||||
category: null,
|
||||
tags: [],
|
||||
suggestedPriceCents: 4500
|
||||
},
|
||||
model: 'claude-sonnet-5',
|
||||
inputTokens: 1000,
|
||||
outputTokens: 200,
|
||||
costMicros: 4000
|
||||
});
|
||||
|
||||
// Non-empty is all that is needed: getAnthropicClient only has to return
|
||||
// something other than null, and the mock above is what answers.
|
||||
process.env.ANTHROPIC_API_KEY = 'sk-ant-not-used';
|
||||
resetAnthropicClient();
|
||||
|
||||
uploads = await fsp.mkdtemp(path.join(os.tmpdir(), 'workerbg-'));
|
||||
process.env.UPLOADS_DIR = uploads;
|
||||
|
||||
stub = http.createServer((req, res) => {
|
||||
req.on('data', () => undefined);
|
||||
req.on('end', () => {
|
||||
res.writeHead(200, { 'Content-Type': 'image/png' });
|
||||
res.end(PNG_BYTES);
|
||||
});
|
||||
});
|
||||
await new Promise<void>((resolve) => stub!.listen(0, '127.0.0.1', resolve));
|
||||
process.env.REMBG_URL = `http://127.0.0.1:${(stub!.address() as AddressInfo).port}`;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
delete process.env.REMBG_URL;
|
||||
delete process.env.ANTHROPIC_API_KEY;
|
||||
resetAnthropicClient();
|
||||
if (stub) {
|
||||
await new Promise<void>((resolve) => stub!.close(() => resolve()));
|
||||
stub = null;
|
||||
}
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await pool.end();
|
||||
await closeDb();
|
||||
});
|
||||
|
||||
/** A queued submission with one real file on disk and the intent set. */
|
||||
async function seedSubmissionWithPhoto(options: { removeBackground: boolean }): Promise<number> {
|
||||
const { rows } = await pool.query<{ id: number }>(
|
||||
`INSERT INTO items (name, status) VALUES ('Submission placeholder', 'pending') RETURNING id`
|
||||
);
|
||||
const itemId = rows[0]!.id;
|
||||
await pool.query(
|
||||
`INSERT INTO item_drafts (item_id, submitter_note, remove_background)
|
||||
VALUES ($1, 'a note', $2)`,
|
||||
[itemId, options.removeBackground]
|
||||
);
|
||||
await pool.query(
|
||||
`INSERT INTO item_images (item_id, image_path, sort_order)
|
||||
VALUES ($1, '/uploads/worker.jpg', 0)`,
|
||||
[itemId]
|
||||
);
|
||||
await fsp.writeFile(path.join(uploads, 'worker.jpg'), JPEG_BYTES);
|
||||
return itemId;
|
||||
}
|
||||
|
||||
async function originalPathOf(itemId: number): Promise<string | null> {
|
||||
const { rows } = await pool.query<{ original_image_path: string | null }>(
|
||||
`SELECT original_image_path FROM item_images WHERE item_id = $1`,
|
||||
[itemId]
|
||||
);
|
||||
return rows[0]?.original_image_path ?? null;
|
||||
}
|
||||
|
||||
describe('background removal after a draft', () => {
|
||||
// The mock has to actually be in play, or the two cases below would both
|
||||
// pass for the wrong reason — a draft that never happened cuts nothing out.
|
||||
it('drafts successfully, which is what the removal step follows', async () => {
|
||||
await seedSubmissionWithPhoto({ removeBackground: false });
|
||||
|
||||
expect(await draftQueued(1)).toEqual({ drafted: 1, failed: 0, skipped: 0 });
|
||||
});
|
||||
|
||||
// Recorded at submission and acted on here, so the sender never waits and a
|
||||
// sidecar that is down cannot fail their upload.
|
||||
it('cuts out the photos when the submitter asked for it', async () => {
|
||||
const itemId = await seedSubmissionWithPhoto({ removeBackground: true });
|
||||
|
||||
await draftQueued(1);
|
||||
|
||||
expect(await originalPathOf(itemId)).toBe('/uploads/worker.jpg');
|
||||
});
|
||||
|
||||
it('leaves the photos alone when they did not', async () => {
|
||||
const itemId = await seedSubmissionWithPhoto({ removeBackground: false });
|
||||
|
||||
await draftQueued(1);
|
||||
|
||||
expect(await originalPathOf(itemId)).toBeNull();
|
||||
});
|
||||
|
||||
// The governing rule: removal is a convenience on top of a draft that was
|
||||
// written correctly. A sidecar failure must never turn a good draft into a
|
||||
// failed one, because the queue is what the admin actually works from.
|
||||
it('leaves the draft ready when the sidecar fails', async () => {
|
||||
const itemId = await seedSubmissionWithPhoto({ removeBackground: true });
|
||||
// Configured, and nothing listening on it.
|
||||
process.env.REMBG_URL = 'http://127.0.0.1:1';
|
||||
|
||||
await draftQueued(1);
|
||||
|
||||
const { rows } = await pool.query<{ state: string }>(
|
||||
`SELECT state FROM item_drafts WHERE item_id = $1`,
|
||||
[itemId]
|
||||
);
|
||||
expect(rows[0]?.state).toBe('ready');
|
||||
expect(await originalPathOf(itemId)).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user