bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

feat(intake): report what a whole-item background removal actually did (#293)

Browse Source 
removeBackgroundsForItem answered void and threw on the first failure, which is enough for the drafting worker — it catches and logs, and a draft is not worth failing over — and not enough for an admin standing in front of a screen who needs to know whether the thing they pressed happened. It now returns a summary: how many photos the item has, how many carry a cut-out, and whether it stopped early.

It still stops at the first failure. Six attempts against a sidecar that is not answering helps nobody, and stopping costs nothing because removeImageBackground skips a photo that already has an original recorded, so a retry resumes rather than starting over. The count is what turns that retry into an informed choice instead of a guess.

Adds restoreOriginalsForItem alongside it. A photo that was never cut out is skipped rather than refused, because the mixed state a partial removal leaves behind is exactly when somebody reaches for this.

The one existing caller does not change: the worker ignores the result, and ignoring a returned value is legal, which is what makes this additive rather than breaking.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bermudalamb
2026-09-04 10:16:04 -05:00
co-authored by Claude Opus 5
parent f42ea70d88
commit 6f3e77fa88
2 changed files with 257 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/intake/backgroundRemoval.ts
+91 -9
View File
@@ -169,21 +169,103 @@ export async function restoreImageOriginal(imageId: number): Promise<void> {
}
/**
* Every photo of one item, in order.
* What a whole-item removal actually did.
*
* Sequential rather than parallel: the sidecar is assumed to handle one
* request at a time, and the worker it runs inside is not in a hurry. A
* failure on one photo stops the rest, and the caller logs it — the item keeps
* whatever was already done, and nothing is left half-written.
* `void` was enough for the drafting worker, which catches and logs and would
* not fail a draft over a background — but not for an admin standing in front
* of the screen, who needs to know whether the thing they pressed happened.
* Three of four is the normal shape of a bad day here, not an exception, and
* the count is what decides whether pressing it again is worth anything.
*/
export async function removeBackgroundsForItem(itemId: number): Promise<void> {
if (!isRembgConfigured()) return;
export interface RemovalSummary {
/** How many images the item has. */
total: number;
/** How many now carry a cut-out, including any that already did. */
removed: number;
/** Whether it stopped early because one of them failed. */
failed: boolean;
}
/**
* What a whole-item restore did.
*
* No `failed`, because restoring cannot fail the way removing can: it is a
* database swap with no sidecar in it, and a photo that was never cut out is
* skipped rather than being an error.
*/
export interface RestoreSummary {
total: number;
/** How many were put back. Photos that were never cut out are not counted. */
restored: number;
}
/** Every photo of one item, in order. */
async function imageIdsFor(itemId: number): Promise<number[]> {
const { rows } = await pool.query<{ id: number }>(
`SELECT id FROM item_images WHERE item_id = $1 ORDER BY sort_order`,
[itemId]
);
for (const row of rows) {
await removeImageBackground(row.id);
return rows.map((row) => row.id);
}
/**
* Cuts out every photo of one item.
*
* Sequential rather than parallel: the sidecar is assumed to handle one request
* at a time, and neither caller is in a hurry.
*
* Stops at the first failure rather than pushing on. Six attempts against a
* sidecar that is not answering helps nobody, and stopping costs nothing
* because `removeImageBackground` skips a photo that already has an original
* recorded — so pressing the button again resumes where this stopped instead of
* starting over. The summary is what makes that retry an informed choice rather
* than a guess.
*
* An unconfigured environment is not a failure, here as everywhere else in this
* feature: nothing was attempted, so nothing went wrong.
*/
export async function removeBackgroundsForItem(itemId: number): Promise<RemovalSummary> {
const imageIds = await imageIdsFor(itemId);
if (!isRembgConfigured()) {
return { total: imageIds.length, removed: 0, failed: false };
}
let removed = 0;
for (const imageId of imageIds) {
try {
await removeImageBackground(imageId);
removed += 1;
} catch (err) {
// Logged rather than thrown. The caller gets the count, which is the
// thing it can act on; the reason belongs in the log, because the admin's
// next move is the same whatever it was.
console.error(`[background-removal] item ${itemId}, image ${imageId}:`, err);
return { total: imageIds.length, removed, failed: true };
}
}
return { total: imageIds.length, removed, failed: false };
}
/**
* Puts every cut-out photo of one item back.
*
* A photo that was never cut out is skipped rather than refused — the mixed
* state a partial removal leaves behind has to be restorable too, and half an
* item is exactly when somebody reaches for this.
*/
export async function restoreOriginalsForItem(itemId: number): Promise<RestoreSummary> {
const imageIds = await imageIdsFor(itemId);
let restored = 0;
for (const imageId of imageIds) {
try {
await restoreImageOriginal(imageId);
restored += 1;
} catch (err) {
// Only "there was nothing to restore" is skipped. Anything else is a real
// failure and belongs to the caller.
if (!(err instanceof NoOriginalToRestoreError)) throw err;
}
}
return { total: imageIds.length, restored };
}
backend/tests/integration/backgroundRemoval.integration.test.ts
+166 -1
View File
@@ -5,7 +5,12 @@ import os from 'os';
import path from 'path';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
import { removeImageBackground, restoreImageOriginal } from '../../src/intake/backgroundRemoval';
import {
removeImageBackground,
restoreImageOriginal,
removeBackgroundsForItem,
restoreOriginalsForItem
} from '../../src/intake/backgroundRemoval';
beforeEach(async () => {
await resetDb();
@@ -250,3 +255,163 @@ describe('putting the original back', () => {
expect((await pathsOf(imageId)).image_path).toBe('/uploads/original.jpg');
});
});
describe('acting on every photo of one item', () => {
/** One item with three photos on disk, which is what an upload leaves. */
async function seedItemWithPhotos(): Promise<{ itemId: number; imageIds: number[] }> {
const { rows } = await pool.query<{ id: number }>(
`INSERT INTO items (name, status) VALUES ('Three views', 'available') RETURNING id`
);
const itemId = rows[0]!.id;
const imageIds: number[] = [];
for (const [index, name] of ['front.jpg', 'back.jpg', 'base.jpg'].entries()) {
const image = await pool.query<{ id: number }>(
`INSERT INTO item_images (item_id, image_path, sort_order)
VALUES ($1, $2, $3) RETURNING id`,
[itemId, `/uploads/${name}`, index]
);
imageIds.push(image.rows[0]!.id);
await fsp.writeFile(path.join(uploads, name), JPEG_BYTES);
}
return { itemId, imageIds };
}
it('cuts out every photo and says how many', async () => {
await startStub(answerWithPng);
const { itemId } = await seedItemWithPhotos();
const summary = await removeBackgroundsForItem(itemId);
expect(summary).toEqual({ total: 3, removed: 3, failed: false });
});
// An item with no photos is not a failure. It is a perfectly ordinary item
// somebody has not photographed yet, and the button should say so rather
// than erroring.
it('reports nothing to do for an item with no photos', async () => {
await startStub(answerWithPng);
const { rows } = await pool.query<{ id: number }>(
`INSERT INTO items (name, status) VALUES ('Unphotographed', 'pending') RETURNING id`
);
expect(await removeBackgroundsForItem(rows[0]!.id)).toEqual({
total: 0,
removed: 0,
failed: false
});
});
// The case the whole summary exists for: the admin needs to know how far it
// got, because the answer decides whether pressing it again is worth it.
it('stops at the first failure and reports how far it got', async () => {
let served = 0;
await startStub((_req, res) => {
served += 1;
// The first photo works; the sidecar dies before the second.
if (served > 1) {
res.writeHead(500);
res.end('boom');
return;
}
res.writeHead(200, { 'Content-Type': 'image/png' });
res.end(PNG_BYTES);
});
const { itemId } = await seedItemWithPhotos();
const summary = await removeBackgroundsForItem(itemId);
expect(summary).toEqual({ total: 3, removed: 1, failed: true });
});
// And the reason stopping early is acceptable: a retry resumes rather than
// starting over, because removeImageBackground skips what it already did.
it('a retry finishes the job without re-cutting what worked', async () => {
let served = 0;
await startStub((_req, res) => {
served += 1;
if (served === 2) {
res.writeHead(500);
res.end('boom');
return;
}
res.writeHead(200, { 'Content-Type': 'image/png' });
res.end(PNG_BYTES);
});
const { itemId } = await seedItemWithPhotos();
const first = await removeBackgroundsForItem(itemId);
expect(first.failed).toBe(true);
const second = await removeBackgroundsForItem(itemId);
expect(second).toEqual({ total: 3, removed: 3, failed: false });
});
// Unconfigured is not a failure anywhere else in this feature and is not one
// here. Nothing was attempted, so nothing went wrong.
it('does nothing, and calls it nothing, when there is no sidecar', async () => {
await startStub(answerWithPng);
const { itemId } = await seedItemWithPhotos();
delete process.env.REMBG_URL;
expect(await removeBackgroundsForItem(itemId)).toEqual({
total: 3,
removed: 0,
failed: false
});
});
});
describe('putting every original back', () => {
it('restores each photo that was cut out', async () => {
await startStub(answerWithPng);
const { rows } = await pool.query<{ id: number }>(
`INSERT INTO items (name, status) VALUES ('Two views', 'available') RETURNING id`
);
const itemId = rows[0]!.id;
for (const [index, name] of ['a.jpg', 'b.jpg'].entries()) {
await pool.query(
`INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`,
[itemId, `/uploads/${name}`, index]
);
await fsp.writeFile(path.join(uploads, name), JPEG_BYTES);
}
await removeBackgroundsForItem(itemId);
const summary = await restoreOriginalsForItem(itemId);
expect(summary).toEqual({ total: 2, restored: 2 });
const { rows: after } = await pool.query<{ image_path: string }>(
`SELECT image_path FROM item_images WHERE item_id = $1 ORDER BY sort_order`,
[itemId]
);
expect(after.map((r) => r.image_path)).toEqual(['/uploads/a.jpg', '/uploads/b.jpg']);
});
// A photo that was never cut out is skipped rather than being an error — the
// mixed state a partial failure leaves behind has to be restorable too.
it('skips photos that were never cut out', async () => {
await startStub(answerWithPng);
const { rows } = await pool.query<{ id: number }>(
`INSERT INTO items (name, status) VALUES ('Mixed', 'available') RETURNING id`
);
const itemId = rows[0]!.id;
for (const [index, name] of ['x.jpg', 'y.jpg'].entries()) {
await pool.query(
`INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`,
[itemId, `/uploads/${name}`, index]
);
await fsp.writeFile(path.join(uploads, name), JPEG_BYTES);
}
// Cut out only the first, leaving the second as it arrived.
const { rows: images } = await pool.query<{ id: number }>(
`SELECT id FROM item_images WHERE item_id = $1 ORDER BY sort_order`,
[itemId]
);
await removeImageBackground(images[0]!.id);
expect(await restoreOriginalsForItem(itemId)).toEqual({ total: 2, restored: 1 });
});
});