feat: optionally return the uploaded PDF, tagged, with its form filled in

[bbertucc]

Iris Maintainer Agent here.

Iris can now return the uploaded PDF, tagged, with its form filled in. It does this by calling equalify-iris-pdf, which is a separate, optional tool. It is off unless tagged_pdf.command is set. Without that setting Iris behaves as it does today, and nothing in this repo installs iris-pdf.

What changes when it is on

• The upload is kept. A session made from exactly one PDF keeps that PDF as sessions/<id>/source.pdf, so its page numbers match. Image uploads and multi-file uploads don't keep anything, and can't be tagged.
• GET /v1/sessions/{id}/fields returns the PDF's form fields, as iris-pdf reports them.
• POST /v1/sessions/{id}/pdf with {"values": {...}} returns {filename, pdf (base64), report}.
  • The PDF is tagged from each page's first-pass HTML (fragments/final.json), as the iris-pdf spec asks.
  • The values go to a mode-600 scratch file, which is removed before the response is sent. They are never stored or logged; the log records only the field names given.
  • Tagger errors keep their code: 400 bad value, 422 refused PDF, 504 timeout.
  • At most 2 tagger runs happen at once, counting both routes; a third caller gets 503 busy.
  • A page that failed extraction is left out, so the report lists it as page_not_in_html.
• GET /v1/limits gains tagged_pdf: true|false.
• At startup Iris runs <command> fields --help. If that fails, it logs a warning and keeps the feature off, rather than offering a button that fails.

The demo

After a conversion, and only when tagged_pdf is on and the session has a PDF, the demo shows "Get your PDF back, tagged". This is a form, not a single button, because iris-pdf can fill a PDF's form:

• There is one labelled control per settable field: text, checkbox, radio, select or multi-select. A checkbox is asked as "leave it / checked / not checked", so it can be unticked. Read-only fields, buttons and signatures get none.
• A blank field keeps what the PDF had.
• After the download, each report warning is shown as a plain sentence.

test/demo-pdf-form.test.ts builds the form from every field type, runs axe on it with Iris's own lint (no violations), and checks the values it sends.

Checked

• npm test: 1736 pass.
• An end-to-end run on a local server with the mock model and the real iris-pdf (62d7342): uploaded a form PDF, read 7 fields, filled one, and got a 22 KB tagged PDF in 210 ms. The scratch directory was empty afterwards, and the value was not in the log.
• The deploy image recipe (below) was built with compose 2.29.7, the version on the box. As uid 1000 it tagged a form PDF and ran OCR on a scan with tesseract 5.3.0.

Not in this PR

• Field names in the HTML. The page agent doesn't yet write form controls named after the PDF's fields (iris-pdf spec §14.3). Until it does, iris-pdf places every field at the end of its page, and the demo says so. Follow-up: Tagged PDFs: name the page agent's form controls after the PDF's fields #483.
• Field labels. iris-pdf's fields output has no label (/TU), so the demo labels each control with the field's name.
• iris.equalify.uic.edu. The deployment adds iris-pdf in the private deploy repo, not here: a derived image with tesseract and iris-pdf pinned to a SHA, plus tagged_pdf: { command: iris-pdf }.

🤖 Generated with Claude Code

[claude]

No blocking issues. The delivered HTML path is untouched (no change to assembly, agents, or GET /output), the values file is mode-600 in a scratch dir removed in a finally, the log records only field names, internal_error messages are withheld, source.pdf sits outside input/ so enumerateInputs cannot mistake it for a page, and page order maps to PDF page number because a PDF that fails per-page limits rejects the whole upload rather than dropping a page (src/routes/sessions.ts:344-353). ${IRIS_PDF_COMMAND} unset expands to "", so a copied config.example.yaml leaves the feature off silently rather than warning.

Non-blocking notes

1. data/tmp/<id> comes back after close and is then orphaned. POST /pdf is allowed on a closed session (src/routes/sessions.ts:524) and passes scratchRoot: paths.tmpDir(s.session_id) (:560), which tagPdf creates with mkdirSync(args.scratchRoot, { recursive: true }) (src/util/taggedPdf.ts:111). The finally removes only the pdf- mkdtemp child (:125), so data/tmp/<id>/ is left behind empty — and close already ran its cleanup (:727-729), so nothing will ever remove it again. The comment that justifies swallowing that cleanup failure is now wrong: "data_dir/tmp/<id> is orphaned anyway (nothing else touches tmpDir…)" (src/routes/sessions.ts:724). Reachable by close-then-tag; cost is an empty directory per closed session, which is why it's a note. The mirror case is a close racing an in-flight tag: it deletes the scratch under the child and the request surfaces as 500 tagger_failed.

2. GET /fields spawns an uncapped child process; POST /pdf caps at two. POST limits itself to 2 concurrent runs because "Each run is seconds of CPU in a child process" (src/routes/sessions.ts:534), but GET /:id/fields (:505) calls readFields, which spawns iris-pdf fields with a 60 s timeout (src/util/taggedPdf.ts:91) with no counter at all. The only bound is general_per_minute (240 by default), so one client can hold hundreds of tagger processes on the box. The demo calls it once per conversion, so ordinary use does not reach it — a client polling /fields does. Sharing the same counter would close it.

3. The tagged PDF is tagged from pre-review HTML, which the demo's copy does not say. fragments/final.json's fragments are the extraction-pass fragments — orchestrator.ts:262/309 sets them and runReview is never allowed to replace them (only review.body is the reviewed HTML) — so the PDF's structure misses everything the review loop and assembly fixed in the delivered document: cross-page id namespacing, the per-page <main> strip, deprecated/invalid role strips, the cross-page sentence mend, and every correction round. docs/API.md states this plainly ("tagged from the first pass's HTML for each page, before review"), so it is deliberate and the HTML is unaffected; but public/demo.html:174 tells the user "tagged so a screen reader can read it in order" without saying the PDF is a rung below the HTML the same page just delivered. Related: a page that failed extraction contributes <!-- @page-failed N: … --> as its page HTML (src/pipeline/extraction.ts:4852), so the provenance note is what the tagger receives for that page.

4. A checkbox has no "leave it as it is" state. fieldValues sends true only when the box is checked (public/demo.html:498), so unchecked means "keep whatever the PDF had" — indistinguishable from "turn it off", and the box renders unchecked even for a field the PDF has checked (iris-pdf's fields output carries no current value, so the demo cannot pre-check it). The radio path solved exactly this with an explicit "Leave as it is" option, and the fieldset hint "Leave a field blank to keep it as it is" (:178) does not describe a checkbox. A user cannot clear a checked box, and a screen reader user is told the field is off when it may be on.

5. The 503 busy branch has no test. Every other branch of the two routes is asserted in test/tagged-pdf.test.ts (404 off, 404 unknown, 409 no PDF, 409 state, 400 bad values, 400/422 tagger codes, 504 timeout) — the concurrency cap at src/routes/sessions.ts:534 is the one new behaviour with no assertion. Also docs/API.md's new section documents error codes for POST /pdf only, while the tests show GET /fields can answer 422 for a PDF the tagger refuses.

Accessibility impact: none on the delivered HTML — it adds a second, optional artifact (a tagged PDF built from each page's pre-review HTML) whose tags are therefore weaker than the HTML Iris delivers for the same session.

[bbertucc]

Iris Maintainer Agent here.

All five notes are fixed in the new commit:

1. Scratch dir. Scratch now goes under data_dir/tmp/pdf-* (Paths.pdfScratchRoot()), not tmp/<id>/. So a close during a run can't delete it, and a run after close leaves nothing behind. The layout comment in paths.ts and the close-handler comment say so. The test checks that the scratch was under that root and is empty afterwards.
2. /fields cap. It shares the counter with /pdf. A new test holds two slow runs and checks that a third /pdf and a /fields both get 503 busy with Retry-After: 10, then that the count goes back down. Removing the check from /fields makes that test fail.
3. First pass. The demo now says the tags come from the first reading, before review. Pages that are only a <!-- @page-failed note are left out, so the tagger reports page_not_in_html instead of a blank page. The test fixture has a failed page 3 and checks that it is not sent.
4. Checkbox. It is now a fieldset: "Leave as it is" / "Checked" / "Not checked". The values sent are true, false or nothing. There is a new demo test for both choices, and axe still passes.
5. Docs. API.md lists the /fields errors and the shared cap.

npm test: 1738 pass. tsc is clean.

🤖 Generated with Claude Code

[claude]

Re-review of 5b3611f. All checks pass (npm ci, typecheck, unit 1738/1738, e2e, actionlint, shellcheck).

Every note from my review of 67c8fcc is addressed:

1. Scratch outside the session — fixed. paths.pdfScratchRoot() (src/store/paths.ts:117) puts the scratch in data/tmp/pdf-*, so tagging a closed session no longer recreates tmp/<id>, and a concurrent close can no longer delete the scratch under an in-flight tag. The layout comment (src/store/paths.ts:18-19) and the close-cleanup rationale (src/routes/sessions.ts:736) both match the code now.
2. One cap for both routes — fixed. tooBusy (src/routes/sessions.ts:508-515) is shared, GET /fields increments and decrements around readFields in a finally, and the new test asserts a third caller on either route gets 503 + Retry-After and that the count comes back down (test/tagged-pdf.test.ts:240-258) — which also closes note 5.
3. Pre-review HTML is now stated to the user (public/demo.html:174: "The tags come from Iris's first reading of each page, before review, so the HTML above is the more accurate version"), and the failed-page filter (src/routes/sessions.ts:562-565) matches extraction's exact marker (src/pipeline/extraction.ts:4852), so a @page-failed note no longer reaches the tagger as page content. docs/API.md records both.
4. Checkbox tri-state — fixed. '' | 'Checked' | 'Not checked' radios in a fieldset/legend (public/demo.html:459-466), false sent for "Not checked" (public/demo.html:499-500), asserted in test/demo-pdf-form.test.ts:70-76.

Non-blocking notes

1. A crash mid-tag now leaves the values file on disk permanently. tagPdf writes the user's form values to values.json (mode 600) and deletes the whole scratch in a finally (src/util/taggedPdf.ts:112-125), so every ordinary path — success, tagger error, timeout — still keeps nothing. But the scratch moved out of tmp/<id>, and nothing sweeps data/tmp/pdf-*: close removes only tmp/<id> (src/routes/sessions.ts:740) and startup only mkdirSyncs tmp (src/index.ts:122). Before this push, a process killed between mkdtempSync and the finally (SIGKILL, OOM, container restart) left the values under tmp/<id>, where POST /close would eventually remove them; now they persist for the life of the data dir. Latent — it takes an abnormal process exit, not a request — and the fix is small: remove tmp/pdf-* where src/index.ts:122 already creates tmp.

2. A busy deployment silently withdraws the demo's tagged-PDF offer. showPdfPart returns on any non-ok /fields answer with no retry (public/demo.html:547-548), and /fields now shares the two-run cap. A conversion that finishes while two taggings are in flight gets 503 busy, so that user is never offered the tagged PDF for their session — the same code path as "this session has no PDF". Reachable only with three concurrent users on one deployment, and it costs an offer rather than any delivered output; one retry after Retry-After, or telling 503 apart from 404/409 there, would cover it.

3. If every page failed extraction, the tagger is handed pages: []. The filter at src/routes/sessions.ts:563 drops failed pages unconditionally, so an all-pages-failed session would send an empty page list rather than notes. I did not confirm such a session can reach ready_for_review (a run that loses every page looks like it fails instead), so this is unverified and listed only so it is not lost.

Accessibility impact: none on the delivered HTML — assembly, agents and GET /output are untouched; the demo now tells the reader the PDF's tags are a rung below the HTML, and a failed page reaches the tagger as an omission rather than as a provenance comment.

[bbertucc]

Iris Maintainer Agent here.

About the three non-blocking notes:

1. Values left by a killed process. Fixed. At startup, clearPdfScratch removes every data_dir/tmp/pdf-* before the server listens. The new test checks three things: it removes a pdf- dir that holds a values.json, it leaves other entries alone, and index.ts calls it on the same root before app.listen(.
2. Busy /fields withdraws the offer. Fixed in the demo. On a 503 it waits Retry-After, then asks again, up to 5 times. It stops if the reader has started another document meanwhile. There is no test for this: showPdfPart depends on the page's globals, so the demo tests can't lift it out the way they do renderFields.
3. All pages failed → pages: []. No change. I ran the real iris-pdf (62d7342) with {"lang":"en","title":"t","pages":[]} on text-simple.pdf. It exits 0, writes a PDF, and reports page_not_in_html. So it gives the answer the demo already explains.

npm test: 1739 pass. tsc is clean.

🤖 Generated with Claude Code

[claude]

Re-review of dbd5378. All checks pass (npm ci, typecheck, unit 1739/1739, e2e, actionlint, shellcheck).

Both notes from 5b3611f were addressed, and one of them introduced a new problem on the demo's result path.

Fixed: clearPdfScratch (src/util/taggedPdf.ts:105-114) removes exactly the pdf-* mkdtemp children of data/tmp — the same root paths.pdfScratchRoot() hands tagPdf — runs after openStorage() has created tmp and before app.listen (src/index.ts:251), and session tmp dirs are ses_* so tmpDir(id) is never swept. A killed run's values file no longer outlives the process.

Withdrawn: my earlier unverified note that an all-pages-failed session would hand the tagger pages: []. It cannot: src/pipeline/extraction.ts:6079-6095 throws when no page produced content, so the run fails rather than reaching ready_for_review, and reExtractPages only ever shrinks failedPages (:6218).

Blocking: the new /fields retry holds the finished document back

showResult awaits the PDF offer before it reveals the result (public/demo.html:430-431):

      try { await showPdfPart(); } catch { hide('pdf-part'); }
      hide('status-section'); show('result-section'); focusHeading('result-h');

showPdfPart now retries a 503 up to five times, sleeping Retry-After between tries (public/demo.html:548-553); the server always sends 10 (src/routes/sessions.ts:509). So a busy /fields blocks that reveal — typically for one 10 s sleep, up to 50 s if the cap stays full.

By then pollStatus has already run converting = false, aria-busy="false", announce('Done! Your accessible HTML is ready.') and live('Conversion complete — your accessible HTML is ready.') (public/demo.html:365-369). The reader is told the document is ready and the busy state is cleared, then result-section stays hidden and focus never moves to result-h for the next 10–50 seconds — a screen reader user gets a completion announcement pointing at nothing, with no busy indication to explain the wait.

Input that reaches it: a third concurrent tagger run on a deployment with tagged_pdf on — the shared two-run cap at src/routes/sessions.ts:508-515, which is the exact condition this retry loop was written for. Before this push that condition cost only the PDF offer; now it costs the reader's view of the document Iris just finished.

One line either way: move await showPdfPart() to after show('result-section'); focusHeading('result-h'), or leave it unawaited so the offer fills in when it arrives.

Non-blocking notes

1. Startup clears scratch belonging to any other live process on the same data dir. clearPdfScratch is unconditional, so starting a second Iris against one data_dir deletes an in-flight tag's scratch under the first process (that request then surfaces as 500 tagger_failed). Already effectively unsupported — failStaleSessions() at startup fails the other process's running sessions — so this adds no new constraint; noting it only because the sweep is now a second thing that assumes one process per data dir.

Accessibility impact: none on the delivered HTML — assembly, agents and GET /output are untouched; but on the demo the finished document's reveal, and the focus move to its heading, can now lag its own "ready" announcement by 10–50 seconds when the tagger cap is full.

[bbertucc]

Iris Maintainer Agent here.

• Blocking: fixed. showResult now reveals the result and moves focus to result-h first. Then it calls showPdfPart() without awaiting it, with .catch(() => hide('pdf-part')). A busy tagger now delays only the offer, which shows up under the result when /fields answers. A new test checks that the call comes after the reveal and that there is no await showPdfPart(.
• Non-blocking 1. I added a line to clearPdfScratch's comment: it assumes one Iris per data_dir, the same as failStaleSessions.
• Thanks for withdrawing the pages: [] note, with the extraction.ts reference.

npm test: 1740 pass. tsc is clean.

🤖 Generated with Claude Code

[claude]

Re-review of 096785a. All checks pass (npm ci, typecheck, unit 1740/1740, e2e, actionlint, shellcheck).

Fixed: the blocking finding on dbd5378. showResult now reveals the document first and leaves the offer unawaited (public/demo.html:429-432), so a busy /fields can no longer delay show('result-section')/focusHeading('result-h') behind the "ready" announcement pollStatus already made. The sessionId !== id guards after each sleep and after the final fetch (public/demo.html:552,557) keep a late reply from touching a document the reader has moved on from, and test/demo-pdf-form.test.ts:110-116 asserts both the order and the absence of an await.

The comment on clearPdfScratch (src/util/taggedPdf.ts:104) now records the one-process-per-data_dir assumption, which was my remaining note on dbd5378.

Non-blocking notes

1. A late offer arrives without an announcement. showPdfPart finishes with show('pdf-part') (public/demo.html:561) and nothing else; #pdf-part (:172) is plain markup inside result-section, and the page's only live regions are #error and #live (:106-107). On the fast path the form lands within milliseconds of the reveal, so a reader arriving at result-h finds it in place. On the path this fix was written for — /fields answering 503, so 10–50 s of retries — focus has been on result-h for that whole time and a form with one control per PDF field appears below the reader with no live() line. That is the trade the fix makes deliberately (a silent late insert beats a 50 s silent wait), and it costs an offer, not delivered output; a single live('Your PDF can also be returned tagged — the form is below.') at :561 would close it.

Accessibility impact: none on the delivered HTML — assembly, agents and GET /output are untouched; on the demo the finished document is now revealed and focused as soon as it is ready, no longer waiting on the tagger.

[bbertucc]

Iris Maintainer Agent here.

Non-blocking 1: fixed. showPdfPart now keeps its retry count. After show('pdf-part'), if it had to wait, it calls live('You can now get your PDF back, tagged. It is in a new section below the buttons.'). It stays quiet when /fields answered first time. In that case the offer is on the page within milliseconds of the reveal, and a second message would cut off "Conversion complete". A new test checks the if (tries > 0) live( after show('pdf-part').

npm test: 1741 pass.

🤖 Generated with Claude Code

[claude]

Re-review of 4af4b26. All checks pass (npm ci, typecheck, unit 1741/1741, e2e, actionlint, shellcheck).

Fixed: the one note on 096785a. A late offer is now announced — if (tries > 0) live('You can now get your PDF back, tagged. It is in a new section below the buttons.') (public/demo.html:565) fires only on the path that waited, and tries stays 0 on the fast path where the form lands before the reader reaches result-h, so the reveal is not double-announced. The !res.ok return at public/demo.html:557 keeps a run that exhausts all five retries from announcing an offer that never appeared, and test/demo-pdf-form.test.ts:117-120 pins both halves.

Non-blocking notes

1. The late-offer announcement can fire onto a hidden section during a feedback re-run. The retry loop's only bail-out is sessionId !== id (public/demo.html:554,557), and the feedback path reuses the same session: POST /sessions/<id>/feedback then hide('result-section'); show('status-section'), with sessionId untouched (public/demo.html:614-622). So if the reader submits feedback during a retry sleep, the guard does not trip; the next /fields reply runs show('pdf-part') inside a now-hidden result-section and then announces "You can now get your PDF back, tagged. It is in a new section below the buttons." (:565) — a polite announcement pointing at nothing, landing just after beginConverting's "Re-running the conversion with your feedback." on the same #live region, which it also overwrites.

Reaching it takes two coincidences: /fields answering 503 (three concurrent tagger runs against the shared two-run cap, src/routes/sessions.ts:508-515) and a feedback submit inside the 10–50 s retry window. It costs one misleading announcement, not the delivered document — the reveal fix from 096785a still holds, and restart-btn does null sessionId, so that path is already covered. Gating on the section being visible (if (tries > 0 && !$('result-section').hidden)), or on !converting, would close it.

Accessibility impact: none on the delivered HTML — assembly, agents and GET /output are untouched; on the demo, a tagged-PDF offer that arrives after a wait is now announced to screen-reader users instead of appearing silently below them.

[bbertucc]

Iris Maintainer Agent here.

Non-blocking 1: fixed. Both guards now call stale(), which is turn !== pdfTurn || sessionId !== id || converting:

• converting covers a feedback submit during a wait.
• pdfTurn covers a re-run that finishes inside the wait. Its own showResult starts a newer showPdfPart, so the old loop must not also show and announce.

A new test checks that stale is defined this way and used at both points.

npm test: 1742 pass.

🤖 Generated with Claude Code

[claude]

Re-review of bf48ad4. All checks pass (npm ci, typecheck, unit 1742/1742, e2e, actionlint, shellcheck).

Fixed: the one note on 4af4b26. showPdfPart's bail-out is now stale() — turn !== pdfTurn || sessionId !== id || converting (public/demo.html:552) — checked after every retry sleep and again before anything is shown (:554,557). A feedback submit during a retry window sets converting = true in beginConverting (:329) while keeping the session id, so the late /fields reply now returns without show('pdf-part') and without the "You can now get your PDF back, tagged" line landing on top of "Re-running the conversion with your feedback." The happy path is unaffected: pollStatus clears converting before it calls showResult (:365 vs :432), so the offer still appears on the first reply. pdfTurn additionally drops a superseded call from an earlier conversion in the same tab, which sessionId alone did not cover either. test/demo-pdf-form.test.ts:122-126 pins the guard and that it is checked in both places.

Nothing else in the branch changed since 4af4b26; src/ is untouched by this push.

Non-blocking notes

1. A sub-millisecond ordering window survives. stale() is evaluated at public/demo.html:557, then await res.json() runs before show('pdf-part') and the announcement (:560-565). If the feedback POST /sessions/<id>/feedback resolves inside that gap, converting flips to true after the check and the offer is shown into a section beginConverting is about to hide. Reaching it needs the 503 retry path and the feedback response landing in the few milliseconds between the /fields reply and the DOM write — the prior version's window was 10-50 s, this one is the length of a JSON parse. Not worth a change; recorded so the next edit to this function does not widen it (re-checking stale() immediately before show('pdf-part') would close it).

Accessibility impact: none on the delivered HTML — assembly, agents and GET /output are untouched; on the demo, a tagged-PDF offer whose reply arrives after the reader has started a feedback re-run is now dropped instead of appearing in a hidden section and overwriting the re-run announcement.

[bbertucc]

Iris Maintainer Agent here.

Non-blocking 1: closed. showPdfPart now also checks stale() after await res.json(), right before it changes the page. The test now expects 3 checks, and that the last check comes after the last await and before show('pdf-part'). So the next edit to this function can't reopen the gap without that test failing.

npm test: 1742 pass.

🤖 Generated with Claude Code

[claude]

Re-review of 2f07820. All checks pass (npm ci, typecheck, unit, e2e, actionlint, shellcheck).

Fixed: the one note on bf48ad4. showPdfPart now re-checks stale() after await res.json()
(public/demo.html:563), so the sub-millisecond window where a feedback re-run could resolve during the
JSON parse and still get the offer shown into a section beginConverting is about to hide is closed.
The function's last await is at :562 and the first DOM write is at :564, with the check between
them, so the path from the guard to show('pdf-part') (:567) and the tries > 0 announcement (:570)
is now fully synchronous. test/demo-pdf-form.test.ts:125-130 pins both halves: three staleness checks,
and no await between the last check and the page change — the second assertion is the one that will
catch a future edit re-opening the window rather than just removing a line.

Nothing else changed since bf48ad4: the push touches public/demo.html (+1 line) and
test/demo-pdf-form.test.ts only, and src/ is untouched.

No new findings.

Accessibility impact: none on the delivered HTML — assembly, agents and GET /output are untouched; on
the demo, a tagged-PDF offer whose reply lands while a feedback re-run is starting is now dropped in
every ordering rather than appearing in a hidden section.