diff --git a/docs/API.md b/docs/API.md index 7e8835a..a399357 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 4836b46..1416ecb 100644 --- a/src/providers/imageLimits.ts +++ b/src/providers/imageLimits.ts @@ -648,6 +648,75 @@ 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 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 +// 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 330f671..986d50f 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)) { @@ -346,14 +359,45 @@ 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, ); - 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); + // 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, + page, + target, + { bytes: smaller.length, width: shrunk?.width, height: shrunk?.height }, + imageLimits, + ); + if (still) throw new PageTooLargeError(still); + refits.push({ + pdf: f.originalname, + page, + long_edge_px: target, + from: `${size.width}x${size.height}`, + }); + p.buffer = smaller; } pages.push(...rendered); } else { @@ -399,6 +443,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 ce04064..016aa3a 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. @@ -172,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. @@ -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 5e5bb31..d5982d7 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 f1ea1d5..6f39d7c 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 0000000..caf9eef --- /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)); +}); diff --git a/test/pdf-links.test.ts b/test/pdf-links.test.ts index 7ea33bd..fe954d2 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 ca655ed..72f3e71 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 }); + } + }, +);