From 07f050cb979ce6374b0d07af085fef669280351d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 29 Aug 2026 11:24:07 +0100 Subject: [PATCH] output takes a format NAME, and says so instead of ignoring you MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `output: { webp: { quality: 82 } }` was accepted and did nothing. It reads perfectly and mirrors how `pipeline` names an operation, so it is what a caller writes from memory — and quietly it meant format fell through to the "webp" default (right answer, wrong reason) while the options were never read, so the quality asked for was not the quality encoded. Ask for `{ png: … }` the same way and you got a WebP with a .png on the end of it. Every other part of this API rejects a name it does not know. This one did not, and what it let through was the kind of failure nobody finds by looking at the output. Also report `needsConversion` from `/v1/image`. Which formats have to go through ImageMagick before sharp can read their pixels is this service's fact, and it is expensive enough to plan around rather than merely absorb: 8.5 s against 1.1 s for the resize and encode after it. A caller building a seven-rung ladder needs to know so it can decode once itself, and a copy of the list kept on its side would go on paying after this build stopped needing it. Co-Authored-By: Claude Opus 5 --- sidecar/index.ts | 10 ++++++++++ sidecar/sharp/decodable.ts | 14 ++++++++++++++ sidecar/sharp/run.ts | 22 ++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/sidecar/index.ts b/sidecar/index.ts index cada431..df7f87e 100644 --- a/sidecar/index.ts +++ b/sidecar/index.ts @@ -6,6 +6,7 @@ import { ffprobe } from "./probe/ffprobe"; import { runSharp, type SharpRequest } from "./sharp/run"; import { runFfmpeg, type FfmpegRequest } from "./ffmpeg/run"; import { safeInputPath } from "./paths"; +import { needsConversion } from "./sharp/decodable"; import sharp from "sharp"; import { createSidecarConnector } from "@@/sidecar/connect"; import { API_URL } from "@@/officer-url.mjs"; @@ -254,6 +255,15 @@ async function route(request: Request): Promise { hasAlpha: meta.hasAlpha ?? false, pages: meta.pages ?? 1, orientation: meta.orientation ?? null, + /* + Reported, because it is a cost a caller can plan around rather than merely absorb. + + True means every operation on this file starts with an ImageMagick conversion — 8.5 s against + 1.1 s for the resize and encode after it. A caller making one derivative should ignore this. A + caller making seven should decode once itself and work from that, which is the difference + between 67 seconds and 16 for one phone photograph. + */ + needsConversion: needsConversion(meta.format ?? ""), }); } diff --git a/sidecar/sharp/decodable.ts b/sidecar/sharp/decodable.ts index 8b62250..b1a4cb3 100644 --- a/sidecar/sharp/decodable.ts +++ b/sidecar/sharp/decodable.ts @@ -24,6 +24,20 @@ import { unprocessable } from "../errors"; /** Formats sharp reports but cannot decode here. Extend only with evidence, not suspicion. */ const NEEDS_CONVERSION = new Set(["heif", "heic", "avif-sequence"]); +/** + * Does this format have to go through ImageMagick before sharp can read its pixels? + * + * Exported because it is expensive enough to plan around, not merely to handle. A caller building + * several derivatives of one file pays this conversion PER REQUEST — measured at 8.5 s against 1.1 s + * for the resize and encode that follow it — so a seven-rung ladder spends most of its time producing + * the same intermediate seven times. Knowing in advance lets that caller decode once itself. + * + * It is a function here rather than a copy of the set over there on purpose: which formats need this + * changes when the build changes, and a consumer holding its own list would keep paying for a + * conversion that had stopped being necessary. + */ +export const needsConversion = (format: string): boolean => NEEDS_CONVERSION.has(format.toLowerCase()); + export type Decodable = { path: string; /** Call when finished. Removes the intermediate, if there was one. */ diff --git a/sidecar/sharp/run.ts b/sidecar/sharp/run.ts index edc3d81..0f35fec 100644 --- a/sidecar/sharp/run.ts +++ b/sidecar/sharp/run.ts @@ -147,6 +147,28 @@ export async function runSharp(body: SharpRequest): Promise { throw invalid("output must be an object", "output"); } + /* + Unknown keys are refused, because the shape of this object is guessable and the wrong guess is + SILENT. + + `output: { webp: { quality: 82 } }` reads perfectly and mirrors how `pipeline` names an operation, + so it is what a caller writes from memory. Accepted quietly it meant: format fell through to the + "webp" default — right answer, wrong reason — and the options were never read, so the quality asked + for was not the quality encoded. Ask for `{ png: … }` the same way and you get a WebP with a .png + on the end of it. + + Every other part of this API rejects a name it does not know. This one did not, and the failure it + let through was the kind nobody finds by looking at the output. + */ + const known = new Set(["format", "options"]); + const stray = Object.keys(output).filter((key) => !known.has(key)); + if (stray.length) { + throw invalid( + `output has no ${stray.map((key) => JSON.stringify(key)).join(", ")}. It takes { "format": "webp", "options": { … } } — the format is a NAME, not a key.`, + "output", + ); + } + const format = output.format ?? "webp"; if (typeof format !== "string" || !OUTPUT_FORMATS[format]) { throw invalid(