Tag a PDF from Iris's HTML, and fill its form fields - #3
Conversation
Several agents share this folder. Each task gets its own worktree under .claude/worktrees/ so their uncommitted changes never collide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review notes on #2: a subagent's worktree branch must be merged back, git clean -ffdx in the main checkout deletes the worktrees under .claude/worktrees/, and scratch/ is ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
iris-pdf takes the original PDF and Iris's per-page HTML and returns the same PDF with a structure tree: the original drawing is kept as an artifact, and an invisible text layer carries Iris's words at the matching positions. Form fields are filled from a values file and tied to Form elements; --flatten bakes them in. Every output is checked before it is written: pages render to the same pixels outside changed fields, and no text is lost or unreadable. What the tool cannot do safely it refuses with a code and one line. Covers milestones M1-M6 of #1. veraPDF in CI (M9) and the Iris side (M7-M8) are separate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A plain Test workflow with no secrets, and the same checks added to the review's context so the reviewer sees their results. Tesseract is installed for the OCR tests, which skip without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This PR changes
|
There was a problem hiding this comment.
Blocking
1. A PDF page that pages.json does not describe has all of its content turned into an artifact, with nothing in the structure tree and no warning. src/tag.ts:96 html.get(i) ?? "" — a missing page silently becomes an empty plan; src/tag.ts:230 const keep = e.mcids || !pageObj.get("Contents").isNull(); keeps the page in written, so src/tag.ts:112 still does page.put("Contents", [...artifactStreams(doc, page), ...]).
Reached with mixed.pdf (2 pages) and a pages.json holding only sourcePage: 1:
=== page 2 StructParents: null
/Artifact BMC q q 306 0 0 396 0 0 cm /Im0 Do Q Q EMC q 3 Tr Q
The page's image is now explicitly decorative and has no structure element, so AT sees an empty page where before it saw untagged content; the report says only textSource: "none" — no warning, and --strict passes. Either refuse/warn for a page with no HTML, or leave such a page untouched (don't wrap it as an artifact).
2. With no title the output still claims PDF/UA-1 and writes an empty dc:title. src/tag.ts:106 setDocumentInfo(doc, lang, title ?? "") after only warn({ code: "no_title" }); src/pdf/metadata.ts:30 always emits <pdfuaid:part>1</pdfuaid:part>. Same run as above (a pages.json with no title, no --title, source Info with no title):
Info title: undefined
XMP: ...<dc:title><rdf:Alt><rdf:li xml:lang="x-default"></rdf:li></rdf:Alt></dc:title><pdfuaid:part>1</pdfuaid:part>...
DisplayDocTitle true with an empty title is a PDF/UA-1 §7.1 failure and shows a blank window title. no_title is also absent from STRICT (src/tag.ts:37), so --strict does not catch it.
3. An internal link (a footnote reference) loses its annotation, which ends up orphaned with no alternate description. src/tag.ts:264 if (n.type === "Link") linkAnnotation(n, elem, e); — <a href="#fn"> is built as Reference (src/html/build.ts:114), so it never claims the page's GoTo link annotation. That annotation then falls to src/tag.ts:214:
if (l.obj.get("Contents").isNull() && l.uri) l.obj.put("Contents", ctx.doc.newString(l.uri));l.uri is "" for a GoTo link (src/tag.ts:302 only reads A/URI), so no /Contents is written. Reached with any PDF that has an internal link annotation plus Iris HTML using <a href="#…">; on a two-paragraph PDF with one GoTo link annot the output has byType: {Document:1, P:1, Reference:1, Note:1, Link:1}, warnings: [{code:"unmatched_link", detail:"internal link"}], and annot Contents: null. So the annotation is (a) tagged by a Link element appended after all page content, i.e. out of reading order, and (b) has no /Contents//Alt — PDF/UA-1 7.18.1/7.18.5 — while the source HTML did mark it up as a link.
4. align() has no bound on words per page: one dense page whose order differs from the HTML's takes minutes and hundreds of MB. src/align/align.ts:14-36 runs Smith–Waterman (Float64Array((n+1)*(m+1)), src/align/align.ts:47) once per unmatched stretch, and re-queues the gaps, so when few tokens are claimed per pass the cost is O(n²·m). MAX_PAGES = 25 (src/pdf/document.ts:11) caps pages, nothing caps words per page. Measured on this branch, HTML tokens vs page tokens in the opposite order:
500 615ms rss 96MB / 1000 5741ms rss 184MB / 1500 17216ms rss 214MB / 2000 44236ms rss 367MB
4000 — still running after 120s
2000 words is an ordinary dense page, and ×25 pages is ~18 minutes of CPU for one document; a pages.json and PDF crafted to share vocabulary but not order pushes it as far as the caller likes (and Float64Array of n·m eventually throws RangeError, surfacing as internal_error). Cap the tokens per alignment (or the matrix size) and report a refusal past it.
Non-blocking notes
src/pdf/widgets.ts:145const want = value === true ? onStates(w.widget)[0] : …— a checkbox widget with no/AP /Ndictionary (legal; viewers synthesise the appearance) has no on-states, sowantisundefinedanddoc.newName(undefined)throwsTypeError: Cannot read properties of undefined (reading 'length')(checked against this repo's mupdf).checkValuesacceptstruefor any checkbox, so--values '{"cb":true}'on such a PDF exits 3 asinternal_errorinstead of abad_valuerefusal..github/workflows/code-review.yml:180-192— the context step now runsnpm ciandnpm test, i.e. PR-authored lifecycle scripts and test code, in the review job. Forpull_requestthis is gated to same-repo heads, but theworkflow_dispatchpath (header comment at :41-43) is now full code execution from a fork inside a job withid-token: writeand the Bedrock role, where before this PR it only ranactionlint. The comment already warns to read the diff first; worth re-confirming that is the intended posture.
Accessibility impact: pages absent from pages.json come back with their content declared decorative and nothing in the structure tree, and a document with no title still claims PDF/UA-1 with an empty dc:title, so the output can be less accessible than its source HTML.
- A page missing from pages.json is left untouched and warned (page_not_in_html). - PDF/UA is declared only when there is a title and every page is tagged. No empty dc:title is written. no_title and page_not_in_html now fail --strict. - An internal link is now Reference > Link, and the Link owns its GoTo annotation. An unmatched link annotation always gets /Contents. A link target keeps its type unless it is a paragraph or list body. - align() now: - refuses pages over 4000 words (too_many_words); - stops at a work budget (alignment_incomplete); - caches word scores and exits the edit distance early. - Checking a checkbox that has no checked appearance is now bad_value. - The review workflow no longer runs a fork's npm ci or tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewing da00906. Of my four blocking findings on 6c581a3: 1 (missing page), 2 (empty title / false PDF/UA claim), 3 (internal link annotation) and 4 (unbounded alignment) are fixed, and both non-blocking notes are addressed. I measured the alignment bound at the new cap: 4000 vs 4000 reversed words now takes 2.3 s and 219 MB peak RSS and returns complete: false, versus "still running after 120 s" before. One hole remains in the fix for finding 1.
Blocking
1. Finding 1 survives through a second door: a page whose pages.json entry yields no words still has all its content turned into an artifact, with nothing in the structure tree, no warning, and a PDF/UA-1 claim.
The fix only covers a page that is absent from pages.json (src/tag.ts:74, if (!html.has(i))). An entry that is present but whose html produces no words and no structure kids takes the old path:
// src/tag.ts:147
let words: PageWord[] = textLayerWords(page);
if (words.length) report.textSource = "pdf-text";
else if (ordered.length) { // <- skipped: ordered is emptyso words stays empty, textSource is "none", and:
// src/tag.ts:222
const keep = e.mcids || !pageObj.get("Contents").isNull();is true on the Contents clause alone, which puts the page in written, so src/tag.ts:112 runs page.put("Contents", [...artifactStreams(doc, page), ...]).
Reached with test/fixtures/scan-300dpi.pdf (one image-only page) and {"lang":"en","title":"T","pages":[{"sourcePage":1,"html":""}]} — the input equalify-iris would produce for a page it read nothing from:
page /Contents: /Artifact BMC q
q 306 0 0 396 0 0 cm /Im0 Do Q
Q EMC
q 3 Tr
Q
StructTreeRoot /K -> Document, whose /K is []
XMP: ...<dc:title>...T...</dc:title><pdfuaid:part>1</pdfuaid:part>...
report.warnings: []
The scan is now explicitly decorative, the structure tree holds a Document with no kids, and the file claims PDF/UA-1 — exit 0, no warning, and --strict passes. Before tagging AT saw untagged content; after, an empty page. This also contradicts the line this PR adds to README.md:80 ("The output declares PDF/UA-1 only when it has a title and every page is tagged"): this page is not tagged and the claim is written anyway.
Note <img alt="…">-only HTML is already handled correctly (emit calls overlay.empty("Figure", …), so e.mcids is 1 and the Figure carries the alt text) — it is specifically HTML that yields zero marked content that goes wrong. Deciding the page is untagged from e.mcids === 0 rather than from html.has(i) would close both doors with one condition, and there is no test for an entry with empty html.
Non-blocking notes
-
A trailing blank page now costs the whole document its PDF/UA-1 claim, a regression introduced by the fix for finding 1/2. On this repo's own
blank-pagefixture (page 2 empty, no HTML sent for it, which is exactly whattest/fixtures/make.ts:103-105calls the normal case):warnings: [... {"code":"page_not_in_html","page":2, ...}] claims UA: false strict: strict: --strict: page_not_in_htmlA genuinely empty page needs no structure, so the output here is conformant, but
!!title && !untagged(src/tag.ts:109) withholds the claim and--strictfails. Any document ending in a blank page — very common — can no longer reach the tool's most conformant output. Excusing a page with no content stream (or with no marks in it) fromuntaggedwould fix it. -
src/tag.ts:213l.obj.put("Contents", ctx.doc.newString(l.uri || "Link to another part of this document"))— the fallback description is hardcoded English and is written whateverlangis, so a French or Japanese document gets an English alternate description read out for every unmatched internal link. The document/Langis in scope at that point.
Accessibility impact: a pages.json entry with empty HTML still declares that page's entire content decorative, leaves the structure tree empty for it, and claims PDF/UA-1 with no warning, so the output is less accessible than the untagged input.
- If the HTML for a page yields no marked content, the page's drawing is no longer hidden as an artifact. The page is left as it was, warned as page_not_tagged, and the document does not claim PDF/UA. - A blank page needs no HTML: it is neither warned nor counted against the PDF/UA claim. - An unmatched link annotation is now described by the words under it, then by its address. The generic phrase comes last, and its Link element is marked English when the document is not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewing d27ba84. My blocking finding on da00906 (a pages.json entry whose HTML holds nothing to tag) is fixed: src/tag.ts:230-235 now decides from e.mcids and leaves the page untouched with page_not_tagged, in STRICT, with a test that asserts the page's content stream is byte-identical. Both non-blocking notes are also fixed (the blank-page claim regression, and the generic link description now prefers the words under the annotation and is marked Lang en). All checks pass.
The blank-page exemption that fixes the claim regression opens one hole.
Blocking
A blank page that carries an annotation is treated as needing no tags, so the annotation stays untagged and the output still claims PDF/UA-1. New in d27ba84.
src/pdf/content.ts:19 looks only at the page's own painting — "annotations aside":
export function drawsNothing(page: mupdf.PDFPage): boolean {
...
page.runPageContents(dev, mupdf.Matrix.identity);so at src/tag.ts:78-81 such a page is neither warned nor counted:
const blank = drawsNothing(page);
if (!blank) warn({ code: "page_not_in_html", ... });
...
if (!blank) untagged++;and src/tag.ts:111 setDocumentInfo(doc, lang, title ?? "", !!title && !untagged) writes the PDF/UA-1 claim. The page never reaches tagPage, so nothing tags its annotations — note that a page in pages.json does not have this problem, because the unmatched-link loop at src/tag.ts:212-221 runs before the e.mcids check.
Reached with this repo's own blank-page.pdf plus one link annotation on page 2, and pages.json holding only page 1 (i.e. the normal case test/fixtures/make.ts describes):
warnings [["duplicate_text_layer",1]]
claims UA: true
annot StructParent: none Contents: null
page2 StructParents: null
The link has no OBJR in the structure tree, no /StructParent and no /Contents — PDF/UA-1 7.18.1 and 7.18.5 — while the file declares conformance. At da00906 the same input warned page_not_in_html and withheld the claim, so the false claim is a regression from this commit. A widget on such a page goes the same way, and only lands in report.form.unresolved (src/tag.ts:99), which has no warning code, so --strict passes as well. Treating a page as blank only when drawsNothing(page) and it has no /Annots closes it, and there is no test for a blank page with an annotation.
Accessibility impact: a blank page carrying a link or widget leaves that annotation out of the structure tree with no description, and the document still claims PDF/UA-1 with no warning.
A page with no HTML is now excused as blank only if it draws nothing and has no annotations. Otherwise its links or fields would stay untagged while the document claimed PDF/UA. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewing e544443. My blocking finding on d27ba84 (a blank page carrying an annotation was excused as needing no tags, leaving the annotation untagged while the document claimed PDF/UA-1) is fixed:
// src/tag.ts:78-79
const annots = page.getObject().get("Annots");
const blank = drawsNothing(page) && !(annots.isArray() && annots.length);so such a page now warns page_not_in_html (which is in STRICT), increments untagged, and setDocumentInfo(..., !!title && !untagged) withholds the claim; test/tag.test.ts:227-233 asserts both on blank-page.pdf plus a link on page 2. I checked the one shape the guard could have missed — an indirect /Annots — and mupdf resolves it (isArray: true, length: 1), so the guard holds there too. All checks pass.
Non-blocking notes
src/tag.ts:79counts any annotation, including the ones PDF/UA-1 exempts from tagging: an annotation with the Hidden or NoView flag (7.18.1) or a Popup (7.18.3). A document whose trailing blank page carries only such an annotation is conformant, but now loses its PDF/UA-1 claim and fails--strict— the same class of over-conservatism as the blank-page claim regression fixed ind27ba84, in the safe direction this time. Skipping hidden annotations and/Subtype /Popupwhen decidingblankwould close it.
Accessibility impact: a blank page's link or widget can no longer sit outside the structure tree under a PDF/UA-1 claim; the remaining gap only withholds a claim a conformant file could have made.
PDF/UA-1 does not tag them (7.18.1, 7.18.3), so a blank page holding only these keeps the document's PDF/UA claim. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewing 719711a. My one non-blocking note on e544443 (a trailing blank page carrying only annotations PDF/UA-1 exempts — Hidden, NoView, Popup — lost the document's PDF/UA claim and failed --strict) is fixed:
// src/tag.ts:124-132
function needTags(annots: mupdf.PDFObject): boolean {
...
if (a.get("Subtype").asName() !== "Popup" && !(flags & (2 | 32))) any = true;I checked the guard's edges on blank-page.pdf with an annotation added to page 2 and re-parsed from the saved bytes. Hidden (F 2), NoView (F 32), an indirect /F, a real-number /F 2.0, and a lone /Subtype /Popup all leave the page blank (no page_not_in_html, pdfuaid:part written); a visible link, an indirect /Annots, a Text annotation with its Popup, and an annotation with no /Subtype (so asName() returns "") all still warn and withhold the claim. Bits 2 and 6 are the right ones. All checks pass.
No blocking issues.
Non-blocking notes
-
src/tag.ts:127-130dereferences every/Annotselement, so a null entry crashes it. Withblank-page.pdfwhose page 2 has/Annots [null](saved, re-parsed — a well-formed file;pdf_array_gethands back a null object whose JS wrapper has no_doc):TypeError: Cannot read properties of null (reading '_fromPDFObjectKeep') at PDFObject.get (mupdf.js:2353) at needTags (src/tag.ts:129)New in this commit —
annots.isArray() && annots.lengthnever touched an element. Failure is loud and safe:src/cli.ts:94-95turns it intointernal_error, exit 3, nothing written, so this is only a bad diagnostic on a degenerate PDF, not lost output.linkAnnots(src/tag.ts:355-356) has the same shape on a page that is inpages.json, so anisDictionary()skip in both places covers it. A dangling/Annots [99 0 R]is fine — mupdf leaves that non-null. -
test/tag.test.ts:236-241covers onlyF 2. ThePopupand NoView halves ofneedTagshave no test; both work today (I probed them), so nothing is broken, but the exemption list is the part most likely to be edited next.
Accessibility impact: none — a blank page whose only annotations PDF/UA-1 exempts from tagging can now reach the PDF/UA-1 claim, and every annotation that does need tagging still costs the claim.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewing 6fcda07. Both non-blocking notes on 719711a are fixed: needTags skips null entries and so does linkAnnots, and the exemption list now has a test per case.
// src/tag.ts:127
if (!a.isDictionary()) return; // a null entry
// src/tag.ts:357
if (!a.isDictionary() || a.get("Subtype").asName() !== "Link") return;I checked the one way this guard could have gone wrong — real annotations are indirect, so if isDictionary() were false for them needTags would return false for every page and the d27ba84 false-claim bug would be back. On a saved and re-parsed /Annots [9 0 R null] mupdf gives isDictionary: true, isIndirect: true for the annotation and isDictionary: false, isNull: true for the entry, and get() on the null wrapper throws Cannot read properties of null (reading '_fromPDFObjectKeep'), so the guard skips only the null and test/tag.test.ts:236-251 would fail without it. All checks pass.
No blocking issues.
Non-blocking notes
src/tag.ts:357(thelinkAnnotshalf of the fix) has no test. The new loop pushes the null onto page 2 ofblank-page.pdf, andtest/fixtures/blank-page.pages.jsonholds onlysourcePage: 1, so the null is only ever seen byneedTags;linkAnnotsruns only for pages that are inpages.json. The fix is real and reachable — a PDF whose tagged page carries/Annots [null](well-formed; I built one ontext-simple.pdf) tags fine now (Tabs /S,StructParents 0, onlyduplicate_text_layer), and without the guard it is aTypeError→internal_error, exit 3, nothing written. Pushing the null onto a page that is inpages.jsoninstead would cover both halves in the same loop.
Accessibility impact: none — a null /Annots entry no longer decides whether a blank page needs tags, and every annotation that does need tagging still costs the PDF/UA-1 claim.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewing a1a733a. My one non-blocking note on 6fcda07 (the linkAnnots half of the null-/Annots fix had no test, because the null only ever reached needTags) is fixed:
// test/tag.test.ts:263-270
doc.findPage(0).put("Annots", [null]);
tag(doc.saveToBuffer("").asUint8Array().slice(), pagesOf("text-simple"), {}, report);
assert.equal(report.verification.textPreserved, true);I checked that it reaches the guard rather than passing for some other reason. On the saved and re-parsed fixture the entry really is a null array element (isArray: true, length: 1, isDictionary: false, isNull: true, and get() on it throws Cannot read properties of null (reading '_fromPDFObjectKeep')), page 1 of text-simple is in pages.json so tagPage calls linkAnnots, and with !a.isDictionary() removed from src/tag.ts:357 the test fails with that TypeError at linkAnnots (src/tag.ts:356). needTags is not involved on this page (drawsNothing short-circuits), so the two halves of the fix now have a test each. src/tag.ts:239 (.isNull()) and :78 are the only other /Annots readers and neither dereferences an element. All checks pass (typecheck, 65/65 tests).
No blocking issues, and no non-blocking notes.
Accessibility impact: none — test-only; a null /Annots entry on a tagged page is now covered by a regression test, so the page still gets its structure instead of the run aborting.
Refs #1. Builds milestones M1–M6 of the PRD: the
iris-pdfCLI, the refusals, the structure writer, the pixel and text checks, HTML mapping and alignment, scanned pages, and forms.Builds on #2 (its two commits are at the base of this branch; merge #2 first).
What it does
iris-pdf tag --pdf in.pdf --pages pages.json --out out.pdfgives back the same PDF, tagged:/Headers, links owning their annotations, figures with alt text, notes, form fields.--valuesfills text, checkbox, radio and choice fields. Bad values stop the run before anything is written. Values never appear in output, logs or the report.iris-pdf fieldslists the form fields.iris-pdf checkruns veraPDF if installed.Worth a look
src/align/align.ts): repeated local alignment (Smith–Waterman) so Iris's order wins when the PDF's order differs, e.g. two columns.src/pdf/fonts.ts): Helvetica with a CJK fallback, subset in a scratch document and grafted in, so the size cost is ~14–21 KB.Deviations from the PRD
duplicate_text_layer./Off.Tests
53 tests,
npm test: refusals with their codes, field inventory, filling and flattening, reading order against the HTML for every fixture, ParentTree resolution, metadata, links, CJK through ToUnicode, OCR (skips without Tesseract), the verification gate end to end, CLI exit codes, values never echoed, and a run with every network API stubbed to throw.CI: a new
Testworkflow (no secrets), and the same checks added to the review context.🤖 Generated with Claude Code