From 9785964dfadc0b561ac7facedae05ae2aee22bd2 Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:36:15 -0400 Subject: [PATCH 1/4] feat(tagged-pdf): show the page agent the PDF's form field names (#483) With tagged PDFs on, the upload reads the kept PDF's fields (`iris-pdf fields`) into fields.json by page. The page prompt lists them and asks for each control to be named after its field. A field with no named control is logged (`page_fields_missing`), not corrected yet. Co-Authored-By: Claude Opus 5.5 --- docs/API.md | 16 ++++- src/pipeline/context.ts | 3 + src/pipeline/extraction.ts | 21 +++++- src/pipeline/fields.ts | 85 ++++++++++++++++++++++++ src/pipeline/orchestrator.ts | 18 ++++-- src/routes/sessions.ts | 33 +++++++++- src/store/paths.ts | 9 ++- test/form-fields.test.ts | 122 +++++++++++++++++++++++++++++++++++ test/tagged-pdf.test.ts | 27 +++++++- 9 files changed, 318 insertions(+), 16 deletions(-) create mode 100644 src/pipeline/fields.ts create mode 100644 test/form-fields.test.ts diff --git a/docs/API.md b/docs/API.md index a3993573..d0db0d06 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1059,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 **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 +**The index is the whole log.** `src/` emits **124** event types and every one of them has a section +below — **116** sections, because a few cover two or three 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. @@ -1186,6 +1186,7 @@ emits fails it too. | [`contribution_failed`](#contribution_failed) | The filing step threw, **after** `run_complete` | | [`run_failed`](#run_failed) | The run threw, so there is **no document** | | [`tagged_pdf` / `tagged_pdf_failed`](#tagged_pdf--tagged_pdf_failed) | A tagged PDF was made, or could not be | +| [`form_fields` / `page_fields` / `page_fields_missing`](#form_fields--page_fields--page_fields_missing) | The PDF's form field names were read, shown to a page, or not used | | [`calibrate_call_failed`](#calibrate_call_failed) | One calibration verifier call threw — a tool's line, never a run's | ### `page_refit` @@ -4368,6 +4369,17 @@ A [tagged PDF](#get-a-tagged-pdf-optional) was made, or the tagger refused. `ms` never logged. `tagged_pdf_failed` has the tagger's `code` and `error`. An `internal_error`'s message is left out, because it may quote a value. +### `form_fields` / `page_fields` / `page_fields_missing` + +With tagged PDFs on, the upload reads the PDF's form fields, and the page agent is asked to name +each control after its field, so the tagger can tag the field where it sits. + +- `form_fields`: at upload, `fields` (how many) or the tagger's `error` code. Without them the run + converts the same. +- `page_fields`: `image`, `fields` (how many were in the prompt) and `dropped` (past the cap of 40). +- `page_fields_missing`: `image`, and `fields`, the names no control in the HTML has. Only logged; + no correction is made. + ### `calibrate_call_failed` One verifier call in the calibration harness threw: `image` is the page, `defect` the seeded defect the diff --git a/src/pipeline/context.ts b/src/pipeline/context.ts index ba62ebe9..08b81bb6 100644 --- a/src/pipeline/context.ts +++ b/src/pipeline/context.ts @@ -6,6 +6,7 @@ import type { Paths } from "../store/paths.ts"; import type { RunLog } from "../store/runlog.ts"; import type { IrisConfig } from "../config.ts"; import type { PdfLink } from "../util/pdf.ts"; +import type { PdfField } from "../util/taggedPdf.ts"; export interface InputImage { name: string; // filename, e.g. page-001.png @@ -17,6 +18,8 @@ export interface InputImage { // something other than an upload (the regression gate's fixture images) have no // links to give it. links?: PdfLink[]; + // The source PDF's form fields on this page, when tagged PDFs are on (pipeline/fields.ts). + fields?: PdfField[]; } // Everything a pipeline phase needs. Created once per run. diff --git a/src/pipeline/extraction.ts b/src/pipeline/extraction.ts index 6b318fea..c9a9cc79 100644 --- a/src/pipeline/extraction.ts +++ b/src/pipeline/extraction.ts @@ -20,6 +20,7 @@ import { import { examplesForPrompt } from "./memory.ts"; import { altTexts, genericAltProblem, genericAlts } from "./alt.ts"; import { missingLinkProblem, missingLinks, pageLinkContext, unexpectedHrefs } from "./links.ts"; +import { missingFields, pageFieldContext } from "./fields.ts"; import { duplicateIdProblem, duplicateIds, idAudit } from "./anchors.ts"; import { splitWordAudit, splitWordContradictions, splitWordProblem } from "./hyphens.ts"; import { STANDARD as STANDARD_AGENTS, isStandardType, logicalType } from "./contribute.ts"; @@ -3747,9 +3748,19 @@ async function renderPage( ...(redrawn ? { redrawn: true } : {}), }); } + // And its form fields' names, which the image cannot show either (pipeline/fields.ts). + const fields = pageFieldContext(img.fields); + if (fields.shown.length) { + ctx.log.event("page_fields", { + image: img.name, + fields: fields.shown.length, + dropped: fields.dropped, + ...(redrawn ? { redrawn: true } : {}), + }); + } const user = `Convert this document page image (filename: ${img.name}, page ${img.order} of ${ctx.images.length}) ` + - `to accessible HTML.${links.section}${feedbackPreamble(ctx)}${priorSection}`; + `to accessible HTML.${links.section}${fields.section}${feedbackPreamble(ctx)}${priorSection}`; const res = await ctx.router.complete( PAGE_AGENT, "vision", @@ -4326,7 +4337,7 @@ async function correctPage( `Omit it where you are acting on every problem. Return the page in "html" either way — ` + `unchanged where you declined everything — because a reply with no "html" is a reply this run ` + `cannot use, and every problem you did not decline is still to be fixed in the same reply.` + - `${pageLinkContext(img.links).section}`; + `${pageLinkContext(img.links).section}${pageFieldContext(img.fields).section}`; const res = await ctx.router.complete( PAGE_AGENT, "vision", @@ -5021,6 +5032,12 @@ async function extractPage( if (missing.length) { ctx.log.event("page_links_missing", { image: img.name, links: missing.map((l) => l.href) }); } + // Fields with no control named after them are only logged, not corrected: the first + // measurement is whether the names match at all (#483). + const unnamed = missingFields(img.fields, innerHtml); + if (unnamed.length) { + ctx.log.event("page_fields_missing", { image: img.name, fields: unnamed.map((f) => f.name) }); + } // And whether any image on the page was described with a placeholder instead of a // description, checked here for the same reason and on the same terms: it has an exact answer, diff --git a/src/pipeline/fields.ts b/src/pipeline/fields.ts new file mode 100644 index 00000000..fd4ba3b6 --- /dev/null +++ b/src/pipeline/fields.ts @@ -0,0 +1,85 @@ +import type { PdfField } from "../util/taggedPdf.ts"; +import { decodeEntities } from "../util/html.ts"; + +// Naming a PDF's form controls after its own fields (#483). +// +// When tagged PDFs are on, iris-pdf tags each form field where its control sits in the +// HTML, and it finds the control by `name`. The page image does not show field names, so +// the upload reads them from the PDF (`iris-pdf fields`) and they are listed in the page +// agent's prompt, the way links.ts lists link targets. `missingFields` checks the reply. +// A field with no control is logged, not yet corrected. + +export const MAX_FIELDS_PER_PAGE = 40; + +// Choice lists longer than this, or with longer entries, are left out of the prompt. +const MAX_OPTIONS = 10; +const MAX_OPTION_CHARS = 40; +const CHOICE_TYPES = new Set(["radio", "combobox", "listbox"]); + +function describe(f: PdfField): string { + const parts: string[] = []; + // iris-pdf's own type word. Anything else is not printed. + if (/^[a-z]+$/.test(f.type ?? "")) parts.push(f.type); + const options = Array.isArray(f.options) ? f.options : []; + if ( + CHOICE_TYPES.has(f.type) && + options.length > 0 && + options.length <= MAX_OPTIONS && + options.every((o) => typeof o === "string" && o.length <= MAX_OPTION_CHARS) + ) { + parts.push(`options: ${options.map((o) => JSON.stringify(o)).join(", ")}`); + } + return parts.length ? ` (${parts.join("; ")})` : ""; +} + +// The page's form fields, as the section of the page-agent prompt that carries them, plus +// what was dropped to bound it. Empty section when the page has none, so a deployment +// without iris-pdf sends exactly the prompt it sent before. +export function pageFieldContext(fields: PdfField[] = []): { + section: string; + shown: PdfField[]; + dropped: number; +} { + if (fields.length === 0) return { section: "", shown: [], dropped: 0 }; + const shown = fields.slice(0, MAX_FIELDS_PER_PAGE); + const dropped = fields.length - shown.length; + const list = shown + // JSON-quoted: the name is the PDF's, and a quote or newline in it must not end the quote. + .map((f, i) => `${i + 1}. ${JSON.stringify(f.name)}${describe(f)}`) + .join("\n"); + const section = + `\n\n## Form fields on this page (from the source file's own form fields)\n` + + `The source file is a fillable form, and these are its fields on this page. Give each ` + + `field's control a name attribute holding the field's name EXACTLY as listed (the text ` + + `inside the quotes). The names are not labels: keep the label the page prints.\n\n` + + `${list}\n\n` + + (dropped > 0 ? `(…and ${dropped} more field${dropped === 1 ? "" : "s"} on this page.)\n\n` : "") + + `Do not invent names for controls that are not listed. If you cannot tell which control a ` + + `field belongs to, say so in the "log" field.\n`; + return { section, shown, dropped }; +} + +// Every name attribute's value in a fragment of HTML. A scan, like links.ts's `hrefsIn`, +// and preceded by whitespace or a quote so `data-name=` does not count. +function namesIn(html: string): Set { + const found = new Set(); + for (const m of html.matchAll(/(?<=[\s"'])name\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'>]+))/gi)) { + found.add(decodeEntities(m[1] ?? m[2] ?? m[3] ?? "")); + } + return found; +} + +// The listed fields no control in the HTML is named after. Deduplicated by name: a radio +// group is one field with several controls. +export function missingFields(fields: PdfField[] = [], html: string): PdfField[] { + if (fields.length === 0) return []; + const present = namesIn(html); + const seen = new Set(); + const missing: PdfField[] = []; + for (const f of fields.slice(0, MAX_FIELDS_PER_PAGE)) { + if (present.has(f.name) || seen.has(f.name)) continue; + seen.add(f.name); + missing.push(f); + } + return missing; +} diff --git a/src/pipeline/orchestrator.ts b/src/pipeline/orchestrator.ts index 9ccfcc40..44d99eb0 100644 --- a/src/pipeline/orchestrator.ts +++ b/src/pipeline/orchestrator.ts @@ -41,6 +41,7 @@ import { altTexts, genericAlts } from "./alt.ts"; import { markupReport } from "./markup.ts"; import type { Fragment } from "./fragment.ts"; import type { PdfLink } from "../util/pdf.ts"; +import type { PdfField } from "../util/taggedPdf.ts"; // The link annotations the upload extracted from its PDFs, keyed by page order // (see Paths.sessionLinks). Absent for a session of plain images, for a PDF with no @@ -48,11 +49,11 @@ import type { PdfLink } from "../util/pdf.ts"; // which mean the same thing here, so a missing or unreadable file is no links rather // than an error. Links are additive: without them a run produces the document it // always produced. -function readLinks(paths: Paths, sessionId: string): Record { - const path = paths.sessionLinks(sessionId); +// The same holds for form fields (fields.json, #483). +function readByOrder(path: string): Record { if (!existsSync(path)) return {}; try { - const parsed = JSON.parse(readFileSync(path, "utf8")) as Record; + const parsed = JSON.parse(readFileSync(path, "utf8")) as Record; return parsed && typeof parsed === "object" ? parsed : {}; } catch { return {}; @@ -63,13 +64,20 @@ function readLinks(paths: Paths, sessionId: string): Record { // (which is significant — see docs/API.md) survives, independent of filename. export function enumerateInputs(paths: Paths, sessionId: string): InputImage[] { const dir = paths.sessionInput(sessionId); - const links = readLinks(paths, sessionId); + const links = readByOrder(paths.sessionLinks(sessionId)); + const fields = readByOrder(paths.sessionFields(sessionId)); return readdirSync(dir) .filter((f) => f.includes("__")) .map((f) => { const [prefix, ...rest] = f.split("__"); const order = parseInt(prefix, 10); - return { order, name: rest.join("__"), path: join(dir, f), links: links[String(order)] ?? [] }; + return { + order, + name: rest.join("__"), + path: join(dir, f), + links: links[String(order)] ?? [], + fields: fields[String(order)] ?? [], + }; }) .sort((a, b) => a.order - b.order); } diff --git a/src/routes/sessions.ts b/src/routes/sessions.ts index 986d50f8..3433eff1 100644 --- a/src/routes/sessions.ts +++ b/src/routes/sessions.ts @@ -41,7 +41,7 @@ import { shrunkPageRejection, } from "../providers/imageLimits.ts"; import { imageDimensions } from "../util/imageSize.ts"; -import { readFields, tagPdf, taggedPdfCommand, tagTimeoutSeconds, TaggedPdfError } from "../util/taggedPdf.ts"; +import { readFields, tagPdf, taggedPdfCommand, tagTimeoutSeconds, TaggedPdfError, type PdfField } from "../util/taggedPdf.ts"; // 50 MB is a memory bound, not the image limit. It stays well above what an image may // be (see imageLimits.ts) because a PDF legitimately is: 25 pages of scans is a large @@ -157,6 +157,30 @@ export function keepSourcePdf( return true; } +// The kept PDF's form fields, by page, so the page agent can name each control after its +// field (#483, pipeline/fields.ts). Written only when there are some. A failure is returned +// for the run log rather than raised: the run converts the same without them. +export async function keepPdfFields( + command: string, + paths: Paths, + sessionId: string, +): Promise<{ fields: number } | { error: string }> { + let fields: PdfField[]; + try { + fields = await readFields(command, paths.sessionSourcePdf(sessionId)); + } catch (e) { + return { error: e instanceof TaggedPdfError ? e.code : "tagger_failed" }; + } + if (!Array.isArray(fields)) return { error: "tagger_failed" }; + const byPage: Record = {}; + for (const f of fields) { + if (typeof f?.name !== "string" || !Number.isInteger(f.page)) continue; + (byPage[String(f.page)] ??= []).push(f); + } + if (Object.keys(byPage).length) writeFileSync(paths.sessionFields(sessionId), JSON.stringify(byPage, null, 2)); + return { fields: fields.length }; +} + export function sessionsRouter(cfg: IrisConfig, store: Store): Router { const r = Router(); const paths = new Paths(cfg); @@ -442,16 +466,19 @@ export function sessionsRouter(cfg: IrisConfig, store: Store): Router { if (Object.keys(linksByOrder).length) { writeFileSync(paths.sessionLinks(sessionId), JSON.stringify(linksByOrder, null, 2)); } - keepSourcePdf(cfg, paths, sessionId, files); + const formFields = keepSourcePdf(cfg, paths, sessionId, files) + ? await keepPdfFields(taggedPdfCommand(cfg)!, paths, sessionId) + : null; // 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) { + if (refits.length || formFields) { try { const log = new RunLog(paths.sessionLog(sessionId)); for (const r of refits) log.event("page_refit", r); + if (formFields) log.event("form_fields", formFields); } catch { // ignore — observability must not block the run } diff --git a/src/store/paths.ts b/src/store/paths.ts index 5e6124e4..e0ac94cf 100644 --- a/src/store/paths.ts +++ b/src/store/paths.ts @@ -10,8 +10,8 @@ import type { IrisConfig } from "../config.ts"; // final.json), history/ (the PRIOR output.html, snapshotted only when a // feedback re-run is about to overwrite it — not the review loop's rounds), // output.html, log.jsonl (the run log), lint.json, unresolved.md, -// agent-updates.md, links.json, source-name.txt, source.pdf (only with -// tagged PDFs on) +// agent-updates.md, links.json, source-name.txt, source.pdf and +// fields.json (only with tagged PDFs on) // fixtures// and memory/.json — keyed by agent, shared by every session // tmp// one run's scratch. `tmp//agents/` holds agents that session BUILT, // which `loadAgent` prefers over the library for the rest of it. @@ -68,6 +68,11 @@ export class Paths { sessionLinks(id: string): string { return join(this.sessionDir(id), "links.json"); } + // The source PDF's form fields by page order, like links.json (#483). Written only when + // there are some. + sessionFields(id: string): string { + return join(this.sessionDir(id), "fields.json"); + } // The uploaded PDF, kept only when tagged PDFs are on and the upload was one PDF. // It is what `iris-pdf` tags (util/taggedPdf.ts). sessionSourcePdf(id: string): string { diff --git a/test/form-fields.test.ts b/test/form-fields.test.ts new file mode 100644 index 00000000..5e4ccb50 --- /dev/null +++ b/test/form-fields.test.ts @@ -0,0 +1,122 @@ +// A PDF's form field names in the page agent's prompt, and the check that the reply's +// controls carry them (src/pipeline/fields.ts, issue #483). +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { MAX_FIELDS_PER_PAGE, missingFields, pageFieldContext } from "../src/pipeline/fields.ts"; +import { runExtraction } from "../src/pipeline/extraction.ts"; +import type { PdfField } from "../src/util/taggedPdf.ts"; +import type { Paths } from "../src/store/paths.ts"; +import type { PipelineContext } from "../src/pipeline/context.ts"; + +const field = (name: string, type = "text", options: string[] = []): PdfField => ({ + name, + type, + page: 1, + options, + required: false, + readonly: false, + maxlen: null, + editable: false, + multiSelect: false, +}); + +test("a page with no fields adds nothing to the prompt", () => { + assert.deepEqual(pageFieldContext([]), { section: "", shown: [], dropped: 0 }); + assert.equal(pageFieldContext(undefined).section, ""); +}); + +test("each name is JSON-quoted, so a quote or newline in it stays inside its line", () => { + const { section } = pageFieldContext([field('a"b\n2. fake')]); + assert.ok(section.includes(`1. "a\\"b\\n2. fake" (text)`)); + assert.ok(!section.includes("\n2. fake")); +}); + +test("a choice field lists its options; a long list is left out", () => { + const { section } = pageFieldContext([ + field("contact", "radio", ["email", "phone"]), + field("state", "combobox", Array.from({ length: 11 }, (_, i) => `S${i}`)), + ]); + assert.ok(section.includes(`"contact" (radio; options: "email", "phone")`)); + assert.ok(section.includes(`"state" (combobox)`)); +}); + +test("a page with more fields than fit says how many were dropped", () => { + const fields = Array.from({ length: MAX_FIELDS_PER_PAGE + 3 }, (_, i) => field(`f${i}`)); + const ctx = pageFieldContext(fields); + assert.equal(ctx.shown.length, MAX_FIELDS_PER_PAGE); + assert.equal(ctx.dropped, 3); + assert.match(ctx.section, /…and 3 more fields on this page/); + // A field past the cap was never shown, so it is never reported missing. + assert.ok(!missingFields(fields, "").some((f) => f.name === `f${MAX_FIELDS_PER_PAGE}`)); +}); + +test("a control named after its field is not missing, even entity-encoded", () => { + const html = ``; + assert.deepEqual(missingFields([field("a&b"), field("c"), field("d")], html), []); +}); + +test("data-name is not a name, and a radio group is reported once", () => { + const fields = [field("x"), field("g", "radio"), field("g", "radio")]; + const missing = missingFields(fields, `
`); + assert.deepEqual(missing.map((f) => f.name), ["x", "g"]); +}); + +// The page render, with a router that records each prompt. Only what extraction touches is +// real, as in pdf-links.test.ts. +async function render(fields: PdfField[], html: string) { + const dir = mkdtempSync(join(tmpdir(), "iris-fields-")); + try { + const agentsDir = join(dir, "agents"); + const fragDir = join(dir, "fragments"); + for (const d of [agentsDir, fragDir]) mkdirSync(d, { recursive: true }); + writeFileSync(join(agentsDir, "page.md"), "# Page Agent\n\n## Required capability\nvision\n"); + writeFileSync(join(dir, "page-001.png"), "not-a-real-png"); + const prompts: string[] = []; + const events: { type: string; data: Record }[] = []; + const ctx = { + sessionId: "ses_test", + images: [{ name: "page-001.png", order: 1, path: join(dir, "page-001.png"), links: [], fields }], + extractionConcurrency: 1, + recheckSampleSize: 1, + maxReviewIterations: 1, + paths: { + agentsDir, + tmpAgentsDir: () => join(dir, "tmp-agents"), + agentMemory: (agent: string) => join(dir, `mem-${agent.replace(/\.md$/, "")}.json`), + sessionFragments: () => fragDir, + } as unknown as Paths, + router: { + complete: async (_agent: string, _cap: string, messages: { content: string }[]) => { + prompts.push(messages.map((m) => m.content).join("\n")); + return { text: JSON.stringify({ html, log: "" }) }; + }, + }, + log: { event: (type: string, data: Record = {}) => events.push({ type, data }), agentCall: () => {} }, + } as unknown as PipelineContext; + await runExtraction(ctx); + return { prompts, events }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test("the page agent is shown the page's fields, and an unnamed one is logged, not corrected", async () => { + const { prompts, events } = await render( + [field("applicant.name"), field("applicant.consent", "checkbox")], + `

I agree

`, + ); + assert.equal(prompts.length, 1, "a missing field buys no correction pass"); + assert.match(prompts[0], /## Form fields on this page/); + assert.ok(prompts[0].includes(`"applicant.consent" (checkbox)`)); + assert.equal(events.find((e) => e.type === "page_fields")?.data.fields, 2); + assert.deepEqual(events.find((e) => e.type === "page_fields_missing")?.data.fields, ["applicant.consent"]); +}); + +test("a page with no fields sends the prompt it always sent", async () => { + const { prompts, events } = await render([], "

Plain page.

"); + assert.ok(!prompts[0].includes("## Form fields")); + assert.ok(!events.some((e) => e.type.startsWith("page_fields"))); +}); diff --git a/test/tagged-pdf.test.ts b/test/tagged-pdf.test.ts index fb17f96f..9dbd896f 100644 --- a/test/tagged-pdf.test.ts +++ b/test/tagged-pdf.test.ts @@ -10,7 +10,8 @@ import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import type { IrisConfig } from "../src/config.ts"; import type { AuthedRequest } from "../src/auth/middleware.ts"; -import { keepSourcePdf, sessionsRouter } from "../src/routes/sessions.ts"; +import { keepPdfFields, keepSourcePdf, sessionsRouter } from "../src/routes/sessions.ts"; +import { enumerateInputs } from "../src/pipeline/orchestrator.ts"; import { limitsRouter } from "../src/routes/limits.ts"; import { Store } from "../src/store/db.ts"; import { Paths } from "../src/store/paths.ts"; @@ -125,7 +126,29 @@ test("the upload is kept only when tagged PDFs are on and it is one PDF", () => // And the upload route is what calls it, after the session directory exists. const route = readFileSync(join(dirname(FAKE), "..", "..", "src", "routes", "sessions.ts"), "utf8"); const handler = route.slice(route.indexOf('r.post("/", '), route.indexOf("store.createSession(")); - assert.match(handler, /paths\.initSession\(sessionId\);[\s\S]*keepSourcePdf\(cfg, paths, sessionId, files\);/); + assert.match(handler, /paths\.initSession\(sessionId\);[\s\S]*keepSourcePdf\(cfg, paths, sessionId, files\)\s*\?\s*await keepPdfFields\(/); +}); + +test("the kept PDF's form fields reach its pages, and a failure writes nothing", async () => { + const dir = mkdtempSync(join(tmpdir(), "iris-fields-")); + try { + const paths = new Paths(cfg(dir, FAKE)); + paths.initSession("ses_f"); + writeFileSync(paths.sessionSourcePdf("ses_f"), "%PDF"); + writeFileSync(join(paths.sessionInput("ses_f"), "0001__a-p1.png"), "png"); + writeFileSync(join(paths.sessionInput("ses_f"), "0002__a-p2.png"), "png"); + assert.deepEqual(await keepPdfFields(FAKE, paths, "ses_f"), { fields: 2 }); + const [one, two] = enumerateInputs(paths, "ses_f"); + assert.deepEqual(one.fields?.map((f) => f.name), ["applicant.name", "applicant.consent"]); + assert.deepEqual(two.fields, []); + + paths.initSession("ses_e"); + writeFileSync(paths.sessionSourcePdf("ses_e"), "ENCRYPTED"); + assert.deepEqual(await keepPdfFields(FAKE, paths, "ses_e"), { error: "encrypted" }); + assert.equal(existsSync(paths.sessionFields("ses_e")), false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } }); test("GET /fields passes the PDF's fields through", async () => { From 0c2ef00918944b8d336501d378cefd26dfb5929a Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:53:09 -0400 Subject: [PATCH 2/4] fix(fields): drop a field name that could end its attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #500: a PDF field name went verbatim into an instruction to copy it into name="…". Names with a quote, <, >, a backtick or a control character, or over 100 characters, are dropped at upload and counted. The section also says not to add a control the image does not show. Co-Authored-By: Claude Opus 5.5 --- docs/API.md | 5 +++-- src/pipeline/fields.ts | 16 ++++++++++++++-- src/routes/sessions.ts | 14 ++++++++++---- test/fixtures/fake-iris-pdf.mjs | 3 +++ test/form-fields.test.ts | 11 ++++++++++- test/tagged-pdf.test.ts | 8 +++++++- 6 files changed, 47 insertions(+), 10 deletions(-) diff --git a/docs/API.md b/docs/API.md index d0db0d06..2ef8cb5c 100644 --- a/docs/API.md +++ b/docs/API.md @@ -4374,8 +4374,9 @@ message is left out, because it may quote a value. With tagged PDFs on, the upload reads the PDF's form fields, and the page agent is asked to name each control after its field, so the tagger can tag the field where it sits. -- `form_fields`: at upload, `fields` (how many) or the tagger's `error` code. Without them the run - converts the same. +- `form_fields`: at upload, `fields` (how many were kept) and `unusable` (names dropped because a + quote, `<`, `>`, a backtick or a control character could end the attribute, or longer than 100 + characters), or the tagger's `error` code. Without them the run converts the same. - `page_fields`: `image`, `fields` (how many were in the prompt) and `dropped` (past the cap of 40). - `page_fields_missing`: `image`, and `fields`, the names no control in the HTML has. Only logged; no correction is made. diff --git a/src/pipeline/fields.ts b/src/pipeline/fields.ts index fd4ba3b6..f0252d2e 100644 --- a/src/pipeline/fields.ts +++ b/src/pipeline/fields.ts @@ -10,6 +10,17 @@ import { decodeEntities } from "../util/html.ts"; // A field with no control is logged, not yet corrected. export const MAX_FIELDS_PER_PAGE = 40; +export const MAX_NAME_CHARS = 100; + +// Characters that would end the `name="…"` attribute the agent copies a name into, as for +// link hrefs (`UNSAFE_CHARS` in util/pdf.ts). A space is allowed: field names have them, and +// a space cannot leave a quoted value. A field refused here is tagged at the end of its +// page, as every field was before #483. +const UNSAFE_NAME = /["'<>`\u0000-\u001f\u007f]/; + +export function usableFieldName(name: unknown): name is string { + return typeof name === "string" && name.length > 0 && name.length <= MAX_NAME_CHARS && !UNSAFE_NAME.test(name); +} // Choice lists longer than this, or with longer entries, are left out of the prompt. const MAX_OPTIONS = 10; @@ -54,8 +65,9 @@ export function pageFieldContext(fields: PdfField[] = []): { `inside the quotes). The names are not labels: keep the label the page prints.\n\n` + `${list}\n\n` + (dropped > 0 ? `(…and ${dropped} more field${dropped === 1 ? "" : "s"} on this page.)\n\n` : "") + - `Do not invent names for controls that are not listed. If you cannot tell which control a ` + - `field belongs to, say so in the "log" field.\n`; + `Do not add a control the image does not show, and do not invent names for controls that ` + + `are not listed. If you cannot tell which control a field belongs to, say so in the "log" ` + + `field.\n`; return { section, shown, dropped }; } diff --git a/src/routes/sessions.ts b/src/routes/sessions.ts index 3433eff1..527db36f 100644 --- a/src/routes/sessions.ts +++ b/src/routes/sessions.ts @@ -41,6 +41,7 @@ import { shrunkPageRejection, } from "../providers/imageLimits.ts"; import { imageDimensions } from "../util/imageSize.ts"; +import { usableFieldName } from "../pipeline/fields.ts"; import { readFields, tagPdf, taggedPdfCommand, tagTimeoutSeconds, TaggedPdfError, type PdfField } from "../util/taggedPdf.ts"; // 50 MB is a memory bound, not the image limit. It stays well above what an image may @@ -159,12 +160,13 @@ export function keepSourcePdf( // The kept PDF's form fields, by page, so the page agent can name each control after its // field (#483, pipeline/fields.ts). Written only when there are some. A failure is returned -// for the run log rather than raised: the run converts the same without them. +// for the run log rather than raised: the run converts the same without them. A name that +// could end the attribute it is copied into is dropped and counted (`usableFieldName`). export async function keepPdfFields( command: string, paths: Paths, sessionId: string, -): Promise<{ fields: number } | { error: string }> { +): Promise<{ fields: number; unusable: number } | { error: string }> { let fields: PdfField[]; try { fields = await readFields(command, paths.sessionSourcePdf(sessionId)); @@ -173,12 +175,16 @@ export async function keepPdfFields( } if (!Array.isArray(fields)) return { error: "tagger_failed" }; const byPage: Record = {}; + let unusable = 0; for (const f of fields) { - if (typeof f?.name !== "string" || !Number.isInteger(f.page)) continue; + if (!usableFieldName(f?.name) || !Number.isInteger(f.page)) { + unusable++; + continue; + } (byPage[String(f.page)] ??= []).push(f); } if (Object.keys(byPage).length) writeFileSync(paths.sessionFields(sessionId), JSON.stringify(byPage, null, 2)); - return { fields: fields.length }; + return { fields: fields.length - unusable, unusable }; } export function sessionsRouter(cfg: IrisConfig, store: Store): Router { diff --git a/test/fixtures/fake-iris-pdf.mjs b/test/fixtures/fake-iris-pdf.mjs index e34d6f25..bc02045b 100755 --- a/test/fixtures/fake-iris-pdf.mjs +++ b/test/fixtures/fake-iris-pdf.mjs @@ -26,6 +26,9 @@ if (command === "fields") { console.log(JSON.stringify([ { name: "applicant.name", type: "text", page: 1, options: [], required: true, readonly: false, maxlen: 40, editable: false, multiSelect: false }, { name: "applicant.consent", type: "checkbox", page: 1, options: ["Yes"], required: false, readonly: false, maxlen: null, editable: false, multiSelect: false }, + ...(pdf.includes("UNSAFE") + ? [`x" onfocus="alert(1)`, "y".repeat(101)].map((name) => ({ name, type: "text", page: 1, options: [], required: false, readonly: false, maxlen: null, editable: false, multiSelect: false })) + : []), ])); } else if (command === "tag") { const values = JSON.parse(readFileSync(a.values, "utf8")); diff --git a/test/form-fields.test.ts b/test/form-fields.test.ts index 5e4ccb50..284c7fe4 100644 --- a/test/form-fields.test.ts +++ b/test/form-fields.test.ts @@ -5,7 +5,7 @@ import assert from "node:assert/strict"; import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { MAX_FIELDS_PER_PAGE, missingFields, pageFieldContext } from "../src/pipeline/fields.ts"; +import { MAX_FIELDS_PER_PAGE, MAX_NAME_CHARS, missingFields, pageFieldContext, usableFieldName } from "../src/pipeline/fields.ts"; import { runExtraction } from "../src/pipeline/extraction.ts"; import type { PdfField } from "../src/util/taggedPdf.ts"; import type { Paths } from "../src/store/paths.ts"; @@ -34,6 +34,15 @@ test("each name is JSON-quoted, so a quote or newline in it stays inside its lin assert.ok(!section.includes("\n2. fake")); }); +test("a name that could end its attribute, or is too long, is not usable; a space is fine", () => { + for (const bad of [`x" onfocus="y`, "a'b", "ab", "a`b", "a\nb", "a\tb", "", "n".repeat(MAX_NAME_CHARS + 1)]) { + assert.equal(usableFieldName(bad), false, JSON.stringify(bad)); + } + for (const ok of ["Full Name", "applicant.name", "a&b", "n".repeat(MAX_NAME_CHARS)]) { + assert.equal(usableFieldName(ok), true, ok); + } +}); + test("a choice field lists its options; a long list is left out", () => { const { section } = pageFieldContext([ field("contact", "radio", ["email", "phone"]), diff --git a/test/tagged-pdf.test.ts b/test/tagged-pdf.test.ts index 9dbd896f..df884f43 100644 --- a/test/tagged-pdf.test.ts +++ b/test/tagged-pdf.test.ts @@ -137,11 +137,17 @@ test("the kept PDF's form fields reach its pages, and a failure writes nothing", writeFileSync(paths.sessionSourcePdf("ses_f"), "%PDF"); writeFileSync(join(paths.sessionInput("ses_f"), "0001__a-p1.png"), "png"); writeFileSync(join(paths.sessionInput("ses_f"), "0002__a-p2.png"), "png"); - assert.deepEqual(await keepPdfFields(FAKE, paths, "ses_f"), { fields: 2 }); + assert.deepEqual(await keepPdfFields(FAKE, paths, "ses_f"), { fields: 2, unusable: 0 }); const [one, two] = enumerateInputs(paths, "ses_f"); assert.deepEqual(one.fields?.map((f) => f.name), ["applicant.name", "applicant.consent"]); assert.deepEqual(two.fields, []); + // A name that could end the attribute it is copied into, or too long to list, is dropped. + paths.initSession("ses_u"); + writeFileSync(paths.sessionSourcePdf("ses_u"), "UNSAFE"); + assert.deepEqual(await keepPdfFields(FAKE, paths, "ses_u"), { fields: 2, unusable: 2 }); + assert.ok(!readFileSync(paths.sessionFields("ses_u"), "utf8").includes("onfocus")); + paths.initSession("ses_e"); writeFileSync(paths.sessionSourcePdf("ses_e"), "ENCRYPTED"); assert.deepEqual(await keepPdfFields(FAKE, paths, "ses_e"), { error: "encrypted" }); From e1e97595f029e3980088f99a8212bc9fc914c864 Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:43:23 -0400 Subject: [PATCH 3/4] docs(api): say what form_fields' unusable and page_fields_missing count Review note on #500. Co-Authored-By: Claude Opus 5.5 --- docs/API.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/API.md b/docs/API.md index 2ef8cb5c..bc541491 100644 --- a/docs/API.md +++ b/docs/API.md @@ -4374,12 +4374,13 @@ message is left out, because it may quote a value. With tagged PDFs on, the upload reads the PDF's form fields, and the page agent is asked to name each control after its field, so the tagger can tag the field where it sits. -- `form_fields`: at upload, `fields` (how many were kept) and `unusable` (names dropped because a - quote, `<`, `>`, a backtick or a control character could end the attribute, or longer than 100 - characters), or the tagger's `error` code. Without them the run converts the same. +- `form_fields`: at upload, `fields` (how many were kept) and `unusable` (fields dropped: a name with + a quote, `'`, `<`, `>`, a backtick or a control character, a name over 100 characters, or no page + number), or the tagger's `error` code. Without them the run converts the same. - `page_fields`: `image`, `fields` (how many were in the prompt) and `dropped` (past the cap of 40). -- `page_fields_missing`: `image`, and `fields`, the names no control in the HTML has. Only logged; - no correction is made. +- `page_fields_missing`: `image`, and `fields`, the names no control in the first pass's HTML has. + Only logged; no correction is made. A control a later correction removes is not reported here, but + the tagged PDF's report lists it as `field_not_in_html`. ### `calibrate_call_failed` From f4ca16d67cb5a89f76d87671e3c97ac71e7045ed Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:54:11 -0400 Subject: [PATCH 4/4] docs(api): page_fields carries redrawn Review note on #500. Co-Authored-By: Claude Opus 5.5 --- docs/API.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/API.md b/docs/API.md index bc541491..6017d7e3 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1956,7 +1956,7 @@ carried one. `where` is what makes the count attributable: the same character fr from the correction pass and from a specialist are three facts about three different calls — and `redrawn: true` is present when the reply was a page's [second draw](#page_redrawn), whose markup Iris discarded, because a redraw makes two `extract` calls for one page. The same flag appears for the same -reason on `page_style_attributes`, `page_digit_groups` and `page_links`. It is +reason on `page_style_attributes`, `page_digit_groups`, `page_links` and `page_fields`. It is written AFTER `agent_call`, so the reply on record in the round logs is still the model's own — the census behind this row was a $0 regrade of logs already on disk, and a strip applied before the log would have left no way to take that measurement or any future one. @@ -4377,7 +4377,8 @@ each control after its field, so the tagger can tag the field where it sits. - `form_fields`: at upload, `fields` (how many were kept) and `unusable` (fields dropped: a name with a quote, `'`, `<`, `>`, a backtick or a control character, a name over 100 characters, or no page number), or the tagger's `error` code. Without them the run converts the same. -- `page_fields`: `image`, `fields` (how many were in the prompt) and `dropped` (past the cap of 40). +- `page_fields`: `image`, `fields` (how many were in the prompt), `dropped` (past the cap of 40) and, + on a second draw, `redrawn: true`. - `page_fields_missing`: `image`, and `fields`, the names no control in the first pass's HTML has. Only logged; no correction is made. A control a later correction removes is not reported here, but the tagged PDF's report lists it as `field_not_in_html`.