From 703d92deb958e1f2c98cd29b4421360171e46e6c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 00:52:07 +0000 Subject: [PATCH 1/2] fix(uploads): a page too big to send is rendered smaller, not refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rasterizing runs at a fixed DPI, so a page image's pixel count follows the physical page: a letter page lands at 1275x1650 and a 4000 pt fold-out at 8334x8334, past the 8000 px ceiling above which the vision model errors instead of downscaling. The upload route refused the whole document for that, advising the caller to re-export the page smaller — by hand, what the renderer can do exactly. Those pixels were Iris's choice, not theirs. Iris now renders that page again, at a size it can send, and only on the path that was about to fail: a document that converts today is rendered exactly as it was. The target is the long edge the model reads where that is documented, and the `max_dimension_px` ceiling where Iris is working from assumed limits, so no claim is made about a model whose real limits are unknown. Nothing is cropped — only resolution is given up — and the page is recorded as `page_refit` in the session's run log, because it is then read at a lower resolution than the rest of the document and nothing else would say so. A page still over after the second render fails as before, with a sentence added naming the size it was retried at, since the dimensions in that message are now from a render the caller never asked for. Co-Authored-By: Claude Opus 5 Co-authored-by: Rogue-Git-Dev <56422610+Rogue-Git-Dev@users.noreply.github.com> --- docs/API.md | 43 ++++++++++++-- src/providers/imageLimits.ts | 68 +++++++++++++++++++++ src/routes/sessions.ts | 57 +++++++++++++++++- src/util/pdf.ts | 55 +++++++++++++++++ test/e2e.sh | 33 +++++++++++ test/image-limits.test.ts | 84 ++++++++++++++++++++++++++ test/pdf-large-pages.test.ts | 111 +++++++++++++++++++++++++++++++++++ 7 files changed, 443 insertions(+), 8 deletions(-) create mode 100644 test/pdf-large-pages.test.ts diff --git a/docs/API.md b/docs/API.md index 7e8835ac..a3993573 100644 --- a/docs/API.md +++ b/docs/API.md @@ -663,10 +663,18 @@ base64**, which is **3.75 MB on disk**, on Amazon Bedrock. Ask the deployment in assuming, since it moves with the configured model and provider: [`GET /v1/limits`](#upload-limits-unauthenticated). A **PDF** is not measured against that limit — the file you send is not what reaches the model, since Iris rasterizes its pages at its -own resolution — but each *rendered page* is, and a page over it fails with a `400` naming the -page and the PDF. That happens with large-format pages: rasterizing at a fixed DPI means the -page image scales with the physical page, so a letter page renders well inside the limit and an -ARCH-D drawing does not. +own resolution — but each *rendered page* is. That matters for large-format pages: rasterizing +at a fixed DPI means the page image scales with the physical page, so a letter page renders well +inside the limit and an ARCH-D drawing does not. + +A page that renders too large is **rendered again, smaller**, rather than failing the upload — +those pixels are Iris's choice and not yours. The second render fits the long edge the model +reads where that is documented (so nothing it would have looked at is given up), and the +`max_dimension_px` ceiling where Iris has no published limits for the configured model (so only +pixels that could not have been sent at all are given up). The page appears in the session's run +log as [`page_refit`](#page_refit). A page that is *still* over after that — a dense photographic +or halftoned scan is over on bytes, not on size — fails with a `400` naming the page, the PDF, +and the size it was retried at. Pixel dimensions are mostly **not** a limit worth planning around, and this is the common misdiagnosis: a large-but-light image converts fine, while a small-but-heavy photo is what @@ -1051,8 +1059,8 @@ The events worth grepping for have a section each below, and the index is a link the index when you have a `type` off a log line and want to know what it means; read a section when you want to know what the field it names is for and what it costs. -**The index is the whole log.** `src/` emits **120** event types and every one of them has a section -below — **114** sections, because a few cover a pair of events that are only read together. So a +**The index is the whole log.** `src/` emits **121** event types and every one of them has a section +below — **115** sections, because a few cover a pair of events that are only read together. So a `type` you cannot find here is not one the index skipped: it is a misread line, or a name `src/` no longer emits. @@ -1064,6 +1072,7 @@ emits fails it too. | `type` | What it records | | --- | --- | +| [`page_refit`](#page_refit) | A page too large to send was rendered again, smaller, instead of refusing the document | | [`run_queued` / `run_dequeued`](#run_queued--run_dequeued) | The run's wait for a concurrency slot | | [`run_start`](#run_start) | The run's opening line: how many source pages, and which of the three paths it took | | [`phase`](#phase) | The pipeline entered a phase | @@ -1179,6 +1188,28 @@ emits fails it too. | [`tagged_pdf` / `tagged_pdf_failed`](#tagged_pdf--tagged_pdf_failed) | A tagged PDF was made, or could not be | | [`calibrate_call_failed`](#calibrate_call_failed) | One calibration verifier call threw — a tool's line, never a run's | +### `page_refit` + +One page of an uploaded PDF was rendered a second time, smaller, because the first render came out too +large to send: `pdf` and `page` name it (the PDF's own page number, 1-based), `from` is the size the +first render produced (`8334x8334`), and `long_edge_px` is what the second one was fitted to. One line +per page refitted, so a document of fold-outs writes several. + +This is the only event the upload route writes, and it is written before the run is queued — +rasterizing is what measures a page, so the pages have to be too big before there is a session to +record it against. A log that opens with `page_refit` rather than +[`run_queued`](#run_queued--run_dequeued) is therefore reading correctly. + +The line exists because the page it names is read at a lower resolution than the rest of the document, +and nothing else in the log would say so. Rasterizing runs at a fixed DPI, so pixel count follows the +physical page: a letter page lands near 1275x1650 and a 55-inch drawing past 8000 px, above which the +vision model errors instead of downscaling. Those pixels are Iris's choice, not the caller's, so Iris +renders the page again instead of refusing the upload — nothing is cropped, only resolution is given +up. When a fold-out's findings read thinner than the letter pages around it, this is the line that +explains why, and re-exporting that page at a smaller trim size is the fix that gets it back. A page +still over the limit after the second render fails the upload instead, with a `400` that names the size +it was retried at. + ### `run_queued` / `run_dequeued` The run's wait for a concurrency slot: how busy the queue was when it was admitted (`running` of diff --git a/src/providers/imageLimits.ts b/src/providers/imageLimits.ts index 4836b462..d936c70a 100644 --- a/src/providers/imageLimits.ts +++ b/src/providers/imageLimits.ts @@ -648,6 +648,74 @@ export function rasterizedPageRejection( return null; } +// How small to render a rasterized page that does not fit, or null when rendering it +// again cannot help. Read only after `rasterizedPageRejection` has said there is a +// problem; what it answers is whether Iris can fix that problem itself. +// +// It can, because it chose the pixels. A page over either limit at util/pdf.ts's DPI is +// a page whose ink Iris rendered too big — the caller uploaded a PDF, and the remedy the +// rejection offers them (re-export at a smaller page size, re-save the pages as JPEGs) +// asks them to do by hand what the renderer can do exactly. Refusing the document was +// the honest thing to do while the only alternative was failing inside a model call four +// minutes later; it is not the honest thing to do when a second render fits. +// +// WHICH pixels may be given up is the only real question, and the answer differs by +// basis, which is why it is decided here rather than at the renderer: +// +// documented — the long edge is a fact about the configured model: it downscales to +// that size before reading, so rendering to it discards exactly the pixels the model +// was going to discard anyway. This is the same trade `imageLimitsHint` already +// recommends to a caller with an oversized IMAGE, applied by Iris to a page the +// caller never sized. +// assumed — the long edge is a guess, so rendering to it would be throwing away +// detail the model may well have read, which is the quiet damage this module exists +// to avoid. Only the hard ceiling is given up to, and only the pixels above it: they +// cannot be sent to the model at all under Iris's own rule (`dimensionReason`), so +// nothing that could have been read is lost. A page over the BYTE cap alone is +// therefore left to its rejection on an assumed basis — it is already inside the +// ceiling, so this returns null and the caller reports the refusal. +// +// Null for a page whose dimensions could not be read, too: the target is a comparison +// against the long edge this page HAS, and a page whose header would not parse has not +// told us. (pdftoppm writes a well-formed PNG, so this is a guard rather than a case.) +export function refitLongEdge(page: { width?: number; height?: number }, limits: ImageLimits): number | null { + if (page.width === undefined || page.height === undefined) return null; + const target = limits.basis === "documented" ? limits.max_long_edge_px : limits.max_dimension_px; + // A target at or above what the page already is would re-render the same picture and + // reject it twice. + return target < Math.max(page.width, page.height) ? target : null; +} + +// Why a page is refused after Iris has already rendered it smaller, or null if the +// smaller render is fine. +// +// The same two limits and the same advice — the remedies in `rasterizedPageRejection` +// are still the caller's, and a page that is over at `longEdgePx` is over because of +// what is ON it. What this adds is the one thing that message would otherwise be wrong +// about: the dimensions it prints are the SECOND render's, not the one the DPI would +// have produced, and a caller comparing them against their own page would be measuring +// a picture they never asked for. Saying the retry happened also stops the obvious +// reply, which is to ask Iris to try a smaller size. +export function shrunkPageRejection( + pdfName: string, + pageNumber: number, + longEdgePx: number, + page: { bytes: number; width?: number; height?: number }, + limits: ImageLimits, +): string | null { + const why = rasterizedPageRejection(pdfName, pageNumber, page, limits); + if (!why) return null; + // Whose number `longEdgePx` is, on the same terms as `dimensionReason`: on a + // documented basis `refitLongEdge` renders to what the model reads, and on an assumed + // one to the largest side Iris will send at all. Claiming the first where only the + // second is known would be putting a promise about the model in this sentence. + const size = + limits.basis === "documented" + ? `${longEdgePx} px on the long edge, the size the vision model reads` + : `${longEdgePx} px on the long edge, the largest it will send`; + return `${why} Iris rendered this page again at ${size}, and it is still over.`; +} + // The long edge, in pixels, past which a rasterized page is bigger than the paper a // document normally comes on. Letter at util/pdf.ts's 150 DPI is 1650 px and A4 is // 1755; tabloid is 2550, and a drawing or a fold-out is larger still. Used only to diff --git a/src/routes/sessions.ts b/src/routes/sessions.ts index 330f6717..70c5058e 100644 --- a/src/routes/sessions.ts +++ b/src/routes/sessions.ts @@ -12,7 +12,14 @@ import { runPipeline } from "../pipeline/orchestrator.ts"; import type { AuthedRequest } from "../auth/middleware.ts"; import { sendError } from "./errors.ts"; import { summarizeRun } from "../diagnostics.ts"; -import { rasterizePdf, PdfTooLargeError, MAX_PDF_PAGES, type PageImage, type PdfLink } from "../util/pdf.ts"; +import { + rasterizePdf, + rasterizePageToFit, + PdfTooLargeError, + MAX_PDF_PAGES, + type PageImage, + type PdfLink, +} from "../util/pdf.ts"; import { outputBasenameFromUploads, convertedHtmlFilename, safeStem, titledAs } from "../util/outputNames.ts"; import { captureFixtures } from "../pipeline/regression.ts"; import type { Fragment } from "../pipeline/fragment.ts"; @@ -29,7 +36,9 @@ import { IMAGE_MEDIA_TYPES, imageRejection, rasterizedPageRejection, + refitLongEdge, resolveImageLimits, + shrunkPageRejection, } from "../providers/imageLimits.ts"; import { imageDimensions } from "../util/imageSize.ts"; import { readFields, tagPdf, taggedPdfCommand, tagTimeoutSeconds, TaggedPdfError } from "../util/taggedPdf.ts"; @@ -336,6 +345,10 @@ export function sessionsRouter(cfg: IrisConfig, store: Store): Router { // carrying the link annotations on that page — the one part of a PDF that // rasterizing destroys, so it travels alongside the image (see pipeline/links.ts). const pages: PageImage[] = []; + // Pages that only fit after a second, smaller render. Collected rather than logged + // here because the session whose log this belongs in does not exist yet — the pages + // have to be measured before there is anything to record against (below). + const refits: { pdf: string; page: number; long_edge_px: number; from: string }[] = []; try { for (const f of files) { if (PDF_EXT.test(f.originalname)) { @@ -353,7 +366,34 @@ export function sessionsRouter(cfg: IrisConfig, store: Store): Router { { bytes: p.buffer.length, width: size?.width, height: size?.height }, imageLimits, ); - if (why) throw new PageTooLargeError(why); + if (!why) continue; + // Iris chose these pixels, so before refusing the document it renders the + // page again at a size it can send (issue #485). Only this page, and only + // on the path that was about to fail: a document that converts today is + // rendered exactly as it was. + const target = refitLongEdge({ width: size?.width, height: size?.height }, imageLimits); + // A target means the page's dimensions parsed (`refitLongEdge` answers null + // otherwise), and `page` is set on everything `rasterizePdf` returns — but a + // rejection is the safe reading of either being absent, since re-rendering + // needs both the page to ask for and a size to ask for it at. + if (target === null || !size || p.page === undefined) throw new PageTooLargeError(why); + const smaller = await rasterizePageToFit(f.buffer, p.page, target); + const shrunk = imageDimensions(smaller); + const still = shrunkPageRejection( + f.originalname, + i + 1, + target, + { bytes: smaller.length, width: shrunk?.width, height: shrunk?.height }, + imageLimits, + ); + if (still) throw new PageTooLargeError(still); + refits.push({ + pdf: f.originalname, + page: i + 1, + long_edge_px: target, + from: `${size.width}x${size.height}`, + }); + p.buffer = smaller; } pages.push(...rendered); } else { @@ -399,6 +439,19 @@ export function sessionsRouter(cfg: IrisConfig, store: Store): Router { writeFileSync(paths.sessionLinks(sessionId), JSON.stringify(linksByOrder, null, 2)); } keepSourcePdf(cfg, paths, sessionId, files); + // A page the model reads at lower resolution than the rest is a fact about this + // document's output, so the session says so rather than the deployment's stdout: the + // owner of a document whose fold-out reads worse than its letter pages can find out + // why from the log they already have (`GET /v1/sessions/{id}/logs`). Best-effort for + // the same reason `run_queued` is — a run must not fail to start over its own log. + if (refits.length) { + try { + const log = new RunLog(paths.sessionLog(sessionId)); + for (const r of refits) log.event("page_refit", r); + } catch { + // ignore — observability must not block the run + } + } const record = store.createSession({ session_id: sessionId, diff --git a/src/util/pdf.ts b/src/util/pdf.ts index ce04064d..32177ec1 100644 --- a/src/util/pdf.ts +++ b/src/util/pdf.ts @@ -26,6 +26,10 @@ export interface PageImage { // four pixels-worth of words with no target. Extracted separately and handed to // the page agent as ground truth (see pipeline/links.ts). links: PdfLink[]; + // Which page of the PDF this image is, by the document's own numbering — what + // `rasterizePageToFit` needs to render it again. Absent for an uploaded image, + // which is not a page of anything. + page?: number; } // Thrown when a PDF exceeds the page cap, so the route can return a clean 400. @@ -344,8 +348,59 @@ export async function rasterizePdf(pdf: Buffer, originalName: string): Promise

{ + const dir = mkdtempSync(join(tmpdir(), "iris-pdf-fit-")); + try { + const pdfPath = join(dir, "in.pdf"); + writeFileSync(pdfPath, pdf); + const out = join(dir, "pg"); + // Reserved out of the host's render budget like any other shard (see + // `shardsRunning`): this is a pdftoppm process, and a count blind to it would hand + // the core it is using to the next document as well. + shardsRunning += 1; + try { + await execFileP("pdftoppm", [ + "-png", + "-scale-to", + String(longEdgePx), + "-f", + String(page), + "-l", + String(page), + pdfPath, + out, + ]); + } finally { + shardsRunning -= 1; + } + const pngs = readdirSync(dir).filter((f) => f.endsWith(".png")); + // One page in, one image out. Anything else means the range did not mean what this + // thinks it means, and silently returning the first file would hand the caller + // another page's ink under this page's name. + if (pngs.length !== 1) { + throw new Error(`re-rendering page ${page} produced ${pngs.length} images, expected 1`); + } + return readFileSync(join(dir, pngs[0])); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} diff --git a/test/e2e.sh b/test/e2e.sh index 5e5bb31e..d5982d70 100755 --- a/test/e2e.sh +++ b/test/e2e.sh @@ -1120,6 +1120,39 @@ umarkers=$(echo "$uout" | grep -o '\[page not fully transcribed\]' | wc -l | tr && pass "the marker is delivered as content, once, rather than tidied away" \ || fail "unfinished page" "expected 1 marker in the delivered document, found $umarkers" +echo "==> 9j. a page too big to send is rendered smaller, not refused (issue #485)" +# A one-page PDF whose MediaBox is 4000x4000 pt — a 55-inch square fold-out, which is a +# size a drawing or a poster really comes in. Rasterizing happens at a fixed 150 DPI, so +# that page renders to 8334x8334: past the 8000 px ceiling above which the vision model +# errors instead of downscaling, and so the whole upload used to be refused with advice to +# re-export the page at a smaller page size. Those pixels were Iris's choice and not the +# caller's, so Iris makes the smaller render itself and the document converts. +# +# Written from base64 for the same reason as the PNG in step 5 — the bytes have to be +# exact, since a PDF carries the offsets of its own objects. The readable builder for the +# same file is in test/pdf-large-pages.test.ts. +bigpdf=/tmp/iris-e2e-foldout.pdf +printf 'JVBERi0xLjcKMSAwIG9iago8PCAvVHlwZSAvQ2F0YWxvZyAvUGFnZXMgMiAwIFIgPj4KZW5kb2JqCjIgMCBvYmoKPDwgL1R5cGUgL1BhZ2VzIC9LaWRzIFszIDAgUl0gL0NvdW50IDEgPj4KZW5kb2JqCjMgMCBvYmoKPDwgL1R5cGUgL1BhZ2UgL1BhcmVudCAyIDAgUiAvTWVkaWFCb3ggWzAgMCA0MDAwIDQwMDBdIC9SZXNvdXJjZXMgPDwgL0ZvbnQgPDwgL0YxIDUgMCBSID4+ID4+IC9Db250ZW50cyA0IDAgUiA+PgplbmRvYmoKNCAwIG9iago8PCAvTGVuZ3RoIDU2ID4+CnN0cmVhbQpCVCAvRjEgMTIwIFRmIDIwMCAzNzAwIFRkIChJcmlzIGxhcmdlLWZvcm1hdCBwYWdlKSBUaiBFVAplbmRzdHJlYW0KZW5kb2JqCjUgMCBvYmoKPDwgL1R5cGUgL0ZvbnQgL1N1YnR5cGUgL1R5cGUxIC9CYXNlRm9udCAvSGVsdmV0aWNhID4+CmVuZG9iagp4cmVmCjAgNgowMDAwMDAwMDAwIDY1NTM1IGYgCjAwMDAwMDAwMDkgMDAwMDAgbiAKMDAwMDAwMDA1OCAwMDAwMCBuIAowMDAwMDAwMTE1IDAwMDAwIG4gCjAwMDAwMDAyNDMgMDAwMDAgbiAKMDAwMDAwMDM0OSAwMDAwMCBuIAp0cmFpbGVyCjw8IC9TaXplIDYgL1Jvb3QgMSAwIFIgPj4Kc3RhcnR4cmVmCjQxOQolJUVPRgo=' \ + | base64 -d > "$bigpdf" +big=$(curl -s -X POST "${AUTH[@]}" "$BASE/sessions" -F "images=@$bigpdf;filename=foldout.pdf") +BIG=$(echo "$big" | jq -r '.session_id') +echo "$big" | jq -e '.image_count==1' >/dev/null \ + && pass "the large-format PDF is accepted rather than 400'd" \ + || fail "large-format upload" "expected a session with image_count=1, got $big" +# And it converts: the retry is only worth anything if the page it produces is one the +# pipeline can actually send. +await_session_ready "$BIG" "large-format page" +# The session says so, too. A page the model reads at lower resolution than the rest of a +# document is a fact about that document's output, and its owner should not have to guess +# why the fold-out reads worse than the letter pages. `long_edge_px` is deliberately not +# asserted as a number: it is the model's long edge where that is documented and the 8000 px +# ceiling where it is Iris's own guess, and this deployment runs `mock-model`, which Iris has +# no published limits for. `from` is matched loosely because the exact rounding is poppler's. +refit=$(curl -s "${AUTH[@]}" "$BASE/sessions/$BIG/logs" | jq -c 'select(.type=="page_refit")' | head -1) +echo "$refit" | jq -e '.page==1 and .long_edge_px>0 and (.from|test("^8[0-9]{3}x8[0-9]{3}$"))' >/dev/null \ + && pass "the smaller render is recorded in the session's own log ($refit)" \ + || fail "page_refit" "expected a page_refit line naming the page and the size it was rendered at, got '$refit'" + echo "==> 10. ownership isolation (other endpoints reject unknown id)" code=$(curl -s -o /dev/null -w '%{http_code}' "${AUTH[@]}" "$BASE/sessions/ses_doesnotexist") [ "$code" = "404" ] && pass "unknown session => 404" || fail "isolation" "got $code" diff --git a/test/image-limits.test.ts b/test/image-limits.test.ts index f1ea1d55..6f39d7c4 100644 --- a/test/image-limits.test.ts +++ b/test/image-limits.test.ts @@ -19,7 +19,9 @@ import { modelGeneration, rasterizedPageRejection, rawBytesForBase64Cap, + refitLongEdge, resolveImageLimits, + shrunkPageRejection, visionModelWarning, } from "../src/providers/imageLimits.ts"; import { imageDimensions } from "../src/util/imageSize.ts"; @@ -877,6 +879,88 @@ test("a heavy page that is NOT large-format is diagnosed as density, not page si assert.match(unmeasured, /JPEG/); }); +// ----- Rendering a page again instead of refusing the document (issue #485) ----- + +// A page over either limit is a page Iris rendered too big: the caller sent a PDF and +// never chose its pixels. So the refusal is a last resort, and what these pin is the one +// judgement in it — which pixels may be given up, which differs by basis and is the +// difference between "the model was going to discard these anyway" and throwing away +// detail nobody has checked the model does not read. + +const documented = () => + resolveImageLimits(cfg({ default: "bedrock", bedrock: { region: "us-east-1", default_model: SONNET_46 } })); +const assumed = () => + resolveImageLimits(cfg({ default: "bedrock", bedrock: { region: "us-east-1", default_model: QWEN_VL } })); + +test("an oversized page is re-rendered at the size the model reads, when that is known", () => { + const limits = documented(); + // A 55-inch square page — a poster or a fold-out — at util/pdf.ts's 150 DPI. Over the + // one ceiling the model errors on, and refusing it was the whole of issue #485. + assert.equal(refitLongEdge({ width: 8334, height: 8334 }, limits), 1568); + // Not just to the ceiling: on a documented basis the long edge is a fact about the + // model, which downscales to it before reading, so those are pixels it was going to + // discard. Rendering to 8000 would keep 8000 px of a picture read at 1568. + assert.equal(limits.max_long_edge_px, 1568); + // The byte cap reaches the same answer from a letter page — the dense-scan case, where + // the page is inside every dimension and still too heavy. + assert.equal(refitLongEdge({ width: 1275, height: 1650 }, limits), 1568); + // But only while there is something to give up. A page already at or under the size + // the model reads cannot be helped by rendering it again, and the caller gets the + // refusal with its own advice rather than two renders and the same refusal. + assert.equal(refitLongEdge({ width: 1212, height: 1568 }, limits), null); + assert.equal(refitLongEdge({ width: 800, height: 600 }, limits), null); +}); + +test("on a model nobody has published limits for, only the unsendable pixels are given up", () => { + const limits = assumed(); + // The long edge is a guess here, so rendering to it would throw away detail this model + // may well have read — the quiet damage imageLimits.ts exists to avoid. The hard + // ceiling is different: nothing above it can be sent at all, so the pixels above it + // are lost either way, and the document is the only thing left to save. + assert.equal(refitLongEdge({ width: 8334, height: 8334 }, limits), 8000); + // And a page that is over on BYTES alone keeps its refusal: it is already inside the + // ceiling, so there is no size this can name without guessing on the model's behalf. + assert.equal(refitLongEdge({ width: 1275, height: 1650 }, limits), null); +}); + +test("a page whose dimensions did not parse is not re-rendered at a guessed size", () => { + // The target is a comparison against the long edge the page HAS. A header that would + // not parse has not said, and "cannot say" must never become a number here — the same + // rule the rejection itself follows. + assert.equal(refitLongEdge({}, documented()), null); + assert.equal(refitLongEdge({ width: 9000 }, documented()), null); +}); + +test("a page that fits once it is smaller is not refused, and one that still does not says so", () => { + const limits = documented(); + // The poster above, re-rendered: 1568x1568 of the same ink, well inside both limits. + assert.equal( + shrunkPageRejection("poster.pdf", 3, 1568, { bytes: 900_000, width: 1568, height: 1568 }, limits), + null, + ); + // The dense scan that is dense rather than large: smaller, and still over the cap. + // The caller's own remedies are still the ones to print… + const why = shrunkPageRejection("magazine.pdf", 4, 1568, { bytes: 4_100_000, width: 1212, height: 1568 }, limits); + assert.ok(why); + assert.match(why, /Page 4 of magazine\.pdf/); + assert.match(why, /density rather than page size/); + assert.match(why, /JPEG/); + // …and the one thing that message alone would get wrong is corrected: the dimensions + // it prints are a render the caller never asked for, so the sentence says the retry + // happened. Without it the obvious reply is to ask Iris to try a smaller size. + assert.match(why, /rendered this page again at 1568 px on the long edge/); + assert.match(why, /the size the vision model reads/); +}); + +test("the retried size is not attributed to a model nobody has checked", () => { + // Same care as `dimensionReason`: on an assumed basis 8000 px is Iris's own rule, not + // a size the model is known to read, and the sentence may not say otherwise. + const why = shrunkPageRejection("drawing.pdf", 1, 8000, { bytes: 4_100_000, width: 8000, height: 6000 }, assumed()); + assert.ok(why); + assert.match(why, /rendered this page again at 8000 px on the long edge, the largest it will send/); + assert.doesNotMatch(why, /the vision model reads/); +}); + // ----- GET /v1/limits ----- async function serve(router: express.Router) { diff --git a/test/pdf-large-pages.test.ts b/test/pdf-large-pages.test.ts new file mode 100644 index 00000000..caf9eef0 --- /dev/null +++ b/test/pdf-large-pages.test.ts @@ -0,0 +1,111 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { rasterizePdf, rasterizePageToFit } from "../src/util/pdf.ts"; +import { imageDimensions } from "../src/util/imageSize.ts"; + +// A page Iris rendered too big to send (issue #485). +// +// Rasterizing happens at a fixed DPI, so a page image's pixel count follows the PHYSICAL +// page: a letter page lands at 1275x1650 and a 55-inch poster or fold-out at 8334x8334, +// past the one ceiling above which the vision model errors instead of downscaling. The +// upload route used to refuse the whole document for that, with advice — re-export the +// page smaller — that asks the caller to do by hand what the renderer can do exactly. +// They never chose those pixels; Iris did. +// +// These tests are about the renderer's half of the fix: that the page really does come +// out too large at the DPI, and that rendering it again produces a smaller image of the +// SAME page. Which size to ask for is the limits module's decision and is tested in +// test/image-limits.test.ts (`refitLongEdge`), because it turns on whether the model's +// long edge is a documented fact or Iris's guess. + +// Built byte by byte, like test/pdf-links.test.ts's fixture and for the same reason: what +// is under test is what poppler does with a real file, and the property here is a page +// SIZE, which no mock of the subprocess could exercise. +// +// Two pages of deliberately different shapes — a letter page and a 4000x4000 pt square — +// so an assertion can tell which one came back. That is the whole risk in re-rendering +// one page of a document: `-f`/`-l` counting in the PDF's own numbering, not the array +// index the caller happens to hold. +function twoPagePdf(): Buffer { + const ink = (y: number) => `BT /F1 24 Tf 72 ${y} Td (Iris) Tj ET`; + const stream = (s: string) => `<< /Length ${s.length} >>\nstream\n${s}\nendstream`; + const objs: string[] = [ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R 6 0 R] /Count 2 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources << /Font << /F1 5 0 R >> >> " + + "/Contents 4 0 R >>", + stream(ink(700)), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", + // 4000 pt square: a 55-inch fold-out, which is a page size a drawing or a poster + // really comes in. + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 4000 4000] /Resources << /Font << /F1 5 0 R >> >> " + + "/Contents 7 0 R >>", + stream(ink(3900)), + ]; + let body = "%PDF-1.7\n"; + const offsets: number[] = []; + objs.forEach((o, i) => { + offsets.push(body.length); + body += `${i + 1} 0 obj\n${o}\nendobj\n`; + }); + const startxref = body.length; + body += `xref\n0 ${objs.length + 1}\n0000000000 65535 f \n`; + for (const off of offsets) body += `${String(off).padStart(10, "0")} 00000 n \n`; + body += `trailer\n<< /Size ${objs.length + 1} /Root 1 0 R >>\nstartxref\n${startxref}\n%%EOF\n`; + return Buffer.from(body, "latin1"); +} + +function hasPoppler(): boolean { + try { + execFileSync("pdftoppm", ["-v"], { stdio: "ignore" }); + return true; + } catch { + return false; + } +} + +const skip = hasPoppler() ? false : "poppler-utils not installed"; + +test("a large-format page renders past what can be sent, and says which page it is", { skip }, async () => { + const pages = await rasterizePdf(twoPagePdf(), "foldout.pdf"); + assert.equal(pages.length, 2); + // The ordinary page, for contrast: the size the blanket "a rasterized page is modest" + // assumption was reasoning from, and it is fine. + assert.deepEqual(imageDimensions(pages[0].buffer), { width: 1275, height: 1650 }); + // And the reason the document was refused. Not asserted to the pixel — the exact + // rounding is poppler's — but it has to be square and well past the 8000 px ceiling. + const big = imageDimensions(pages[1].buffer); + assert.ok(big, "the rendered page's header did not parse"); + assert.ok(big.width > 8000 && big.height > 8000, `expected a page past 8000 px, got ${big.width}x${big.height}`); + assert.equal(big.width, big.height); + // The PDF's own page number travels with the image, which is what a second render has + // to ask for. Without it the retry would name an index and render another page's ink. + assert.deepEqual( + pages.map((p) => p.page), + [1, 2], + ); +}); + +test("the page that does not fit is rendered again, smaller, and it is the same page", { skip }, async () => { + const pdf = twoPagePdf(); + // To the hard ceiling: what Iris gives up on a model it has no published limits for — + // only the pixels it could not have sent at all. + const toCeiling = imageDimensions(await rasterizePageToFit(pdf, 2, 8000)); + assert.deepEqual(toCeiling, { width: 8000, height: 8000 }); + // To the size a documented model reads. Square, so this is page 2: page 1 at the same + // request is 1212x1568, and a render that quietly took the first page of the range + // would show up here rather than in a delivered document. + const toLongEdge = imageDimensions(await rasterizePageToFit(pdf, 2, 1568)); + assert.deepEqual(toLongEdge, { width: 1568, height: 1568 }); + // Which that page really is, and the aspect ratio held: the page is rendered smaller, + // never cropped, so nothing that was on it is lost beyond resolution. + assert.deepEqual(imageDimensions(await rasterizePageToFit(pdf, 1, 1568)), { width: 1212, height: 1568 }); +}); + +test("asking for a page the document does not have fails rather than returning another", { skip }, async () => { + // pdftoppm exits non-zero for a range past the end, and that has to stay an error: a + // silent fallback to "the first image in the directory" is how a retry hands page 1's + // ink back under page 9's name. + await assert.rejects(() => rasterizePageToFit(twoPagePdf(), 9, 1568)); +}); From 5f56d0316b58ce73319f5d14c87e7b7c783e2bf2 Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:00:15 -0400 Subject: [PATCH 2/2] fix(uploads): a failed re-render keeps the page's refusal; name the PDF's own page From the review of 0019d39: - A rasterizePageToFit failure now answers with the page's 400, not a 422 carrying pdftoppm's command line and a temp path. Tested through the route with a pdftoppm shim. - The 400 message and page_refit use PageImage.page, the number the re-render uses. - The refit comment in imageLimits.ts says the long edge is the strictest agent's. - The shardsRunning comment names rasterizePageToFit. - test/pdf-links.test.ts's field pin (from #497) now lists `page`. Co-Authored-By: Claude Opus 5.5 --- src/providers/imageLimits.ts | 7 ++-- src/routes/sessions.ts | 12 ++++--- src/util/pdf.ts | 2 +- test/pdf-links.test.ts | 9 +++--- test/request-limits.test.ts | 63 +++++++++++++++++++++++++++++++++++- 5 files changed, 80 insertions(+), 13 deletions(-) diff --git a/src/providers/imageLimits.ts b/src/providers/imageLimits.ts index d936c70a..1416ecbe 100644 --- a/src/providers/imageLimits.ts +++ b/src/providers/imageLimits.ts @@ -662,9 +662,10 @@ export function rasterizedPageRejection( // WHICH pixels may be given up is the only real question, and the answer differs by // basis, which is why it is decided here rather than at the renderer: // -// documented — the long edge is a fact about the configured model: it downscales to -// that size before reading, so rendering to it discards exactly the pixels the model -// was going to discard anyway. This is the same trade `imageLimitsHint` already +// documented — the long edge is a fact about the configured models: the strictest +// downscales to that size before reading, so rendering to it discards the pixels that +// model was going to discard anyway. On a deployment whose vision agents differ, a +// model with a larger long edge loses the difference. This is the same trade `imageLimitsHint` already // recommends to a caller with an oversized IMAGE, applied by Iris to a page the // caller never sized. // assumed — the long edge is a guess, so rendering to it would be throwing away diff --git a/src/routes/sessions.ts b/src/routes/sessions.ts index 70c5058e..986d50f8 100644 --- a/src/routes/sessions.ts +++ b/src/routes/sessions.ts @@ -359,10 +359,11 @@ export function sessionsRouter(cfg: IrisConfig, store: Store): Router { // what the model accepts — and the run would then die inside the first // vision call, minutes in, which is the failure this route exists to catch. for (const [i, p] of rendered.entries()) { + const page = p.page ?? i + 1; const size = imageDimensions(p.buffer); const why = rasterizedPageRejection( f.originalname, - i + 1, + page, { bytes: p.buffer.length, width: size?.width, height: size?.height }, imageLimits, ); @@ -377,11 +378,14 @@ export function sessionsRouter(cfg: IrisConfig, store: Store): Router { // rejection is the safe reading of either being absent, since re-rendering // needs both the page to ask for and a size to ask for it at. if (target === null || !size || p.page === undefined) throw new PageTooLargeError(why); - const smaller = await rasterizePageToFit(f.buffer, p.page, target); + // A failed re-render gets the refusal it would have had, not pdftoppm's error. + const smaller = await rasterizePageToFit(f.buffer, p.page, target).catch(() => { + throw new PageTooLargeError(why); + }); const shrunk = imageDimensions(smaller); const still = shrunkPageRejection( f.originalname, - i + 1, + page, target, { bytes: smaller.length, width: shrunk?.width, height: shrunk?.height }, imageLimits, @@ -389,7 +393,7 @@ export function sessionsRouter(cfg: IrisConfig, store: Store): Router { if (still) throw new PageTooLargeError(still); refits.push({ pdf: f.originalname, - page: i + 1, + page, long_edge_px: target, from: `${size.width}x${size.height}`, }); diff --git a/src/util/pdf.ts b/src/util/pdf.ts index 32177ec1..016aa3a4 100644 --- a/src/util/pdf.ts +++ b/src/util/pdf.ts @@ -176,7 +176,7 @@ async function extractPdfLinks(pdfPath: string): Promise> } // Shards currently rendering, across every upload this process is serving. Read and -// written only by `rasterShards` and `rasterizePages`, and only between synchronous +// written only by `rasterShards`, `rasterizePages` and `rasterizePageToFit`, and only between synchronous // statements — Node runs one of those at a time, so the reserve-then-spawn in // `rasterizePages` cannot interleave with another document's and hand out the same // cores twice. diff --git a/test/pdf-links.test.ts b/test/pdf-links.test.ts index 7ea33bd1..fe954d2e 100644 --- a/test/pdf-links.test.ts +++ b/test/pdf-links.test.ts @@ -980,15 +980,16 @@ test("anchor text is JSON-quoted in the page agent's link list", () => { assert.ok(!section.includes("\n## New rule"), section); }); -// Every field of a page reaches the page agent. A new one fails here until it is handled like -// `name` (safeStem) and `links` (links.ts) are. +// A new page field fails here until it is checked for what reaches a prompt: `name` is filtered +// (safeStem), `links` quoted (links.ts), and `page` is an integer only the upload route reads. test( - "a rasterized page carries only a filtered name, its image and its links", + "a rasterized page carries only a filtered name, its image, its links and its page number", { skip: hasPoppler() ? false : "poppler-utils not installed" }, async () => { const pages = await rasterizePdf(linkPdf(), 'Say "hi"\nnow.PDF'); for (const p of pages) { - assert.deepEqual(Object.keys(p).sort(), ["buffer", "links", "name"]); + assert.deepEqual(Object.keys(p).sort(), ["buffer", "links", "name", "page"]); + assert.ok(Number.isInteger(p.page), String(p.page)); assert.match(p.name, /^[A-Za-z0-9._-]+-p\d+\.png$/); } }, diff --git a/test/request-limits.test.ts b/test/request-limits.test.ts index ca655ede..72f3e71e 100644 --- a/test/request-limits.test.ts +++ b/test/request-limits.test.ts @@ -29,7 +29,8 @@ import type { AuthedRequest } from "../src/auth/middleware.ts"; import { limitsRouter } from "../src/routes/limits.ts"; import { sessionsRouter } from "../src/routes/sessions.ts"; import { Store } from "../src/store/db.ts"; -import { mkdtempSync, readdirSync, rmSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { chmodSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -701,3 +702,63 @@ test("an uploaded image's name is reduced to filename characters before it is st srv.close(); } }); + +// One 4000 pt square page: 8334 px at util/pdf.ts's DPI, over the 8000 px ceiling, so the +// route renders it again smaller (issue #485). +function foldoutPdf(): Buffer { + const objs = [ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 4000 4000] >>", + ]; + let body = "%PDF-1.7\n"; + const offsets: number[] = []; + objs.forEach((o, i) => { + offsets.push(body.length); + body += `${i + 1} 0 obj\n${o}\nendobj\n`; + }); + const startxref = body.length; + body += `xref\n0 ${objs.length + 1}\n0000000000 65535 f \n`; + for (const off of offsets) body += `${String(off).padStart(10, "0")} 00000 n \n`; + body += `trailer\n<< /Size ${objs.length + 1} /Root 1 0 R >>\nstartxref\n${startxref}\n%%EOF\n`; + return Buffer.from(body, "latin1"); +} + +function realPdftoppm(): string | null { + try { + return execFileSync("sh", ["-c", "command -v pdftoppm"], { encoding: "utf8" }).trim() || null; + } catch { + return null; + } +} + +test( + "a re-render that fails gets the page's refusal, not pdftoppm's error", + { skip: realPdftoppm() ? false : "poppler-utils not installed" }, + async () => { + // A pdftoppm that renders normally but fails the smaller re-render (`-scale-to`). + const bin = mkdtempSync(join(tmpdir(), "iris-fake-poppler-")); + writeFileSync( + join(bin, "pdftoppm"), + `#!/bin/sh\nfor a in "$@"; do [ "$a" = "-scale-to" ] && exit 1; done\nexec "${realPdftoppm()}" "$@"\n`, + ); + chmodSync(join(bin, "pdftoppm"), 0o755); + const path = process.env.PATH; + process.env.PATH = `${bin}:${path}`; + const srv = await serveUploadRoute(); + try { + const form = new FormData(); + form.append("images", new Blob([new Uint8Array(foldoutPdf())], { type: "application/pdf" }), "foldout.pdf"); + const res = await fetch(srv.url, { method: "POST", body: form }); + const { error: body } = (await res.json()) as { error: { code: string; message: string } }; + assert.equal(res.status, 400, body.message); + assert.equal(body.code, "invalid_request"); + assert.match(body.message, /^Page 1 of foldout\.pdf renders to 8334x8334 px/); + assert.doesNotMatch(body.message, /pdftoppm|iris-pdf-fit/); + } finally { + process.env.PATH = path; + srv.close(); + rmSync(bin, { recursive: true, force: true }); + } + }, +);