feat(pdf): a flat form's fill-in list comes from the fields tagging makes

[bbertucc]

Iris Maintainer Agent here.

A scanned or flat PDF form has no fields, so the demo had nothing to fill in. iris-pdf now makes fields (issue equalify-iris-pdf#13, merged in #16 as 4533bdf) from the HTML's controls. They're named the same way on every run, from the control's name, id or label.

Change: GET /sessions/{id}/fields checks for a source PDF with no fields whose extracted HTML has an input, select or textarea. For one, it tags the PDF once with no values and lists the fields on the output. POST /pdf fills them by those names, so the demo needs no new code. The real tagger refuses a value for an unknown name, which is why the names come from its own output.

• It runs only when the session is extracted. A PDF with its own fields is never tagged here.
• If the tag fails, the list is empty and the download shows the error.
• It counts against the same two-at-a-time tagger limit.
• The pages-input code moved into tagInput, shared by both routes. Its parse stays inside each route's try.

Checked:

• With the real iris-pdf 4533bdf, a scanned charge-account form (0 source fields) lists 73 fields in about 5 s. Filling all 73 sets 73, the text check passes, and the output opens in macOS PDFKit.
• npm test 1782/1782; e2e.sh all passed.
• All 6 mutations of the new guards are caught.

The owner approved this follow-up to equalify-iris-pdf#13 in session; there's no maintainer-labelled issue. Deploying needs IRIS_PDF_REF at or after 4533bdf.

🤖 Generated with Claude Code

[claude]

All six checks pass. The /pdf refactor into readyToTag/tagInput is behaviour-preserving, the
discovery tag holds a slot under the existing two-at-a-time counter, and the scratch dir is still
removed in inScratch's finally (the read callback is awaited, so readFields runs before the
rmSync). Nothing in the delivered HTML changes. Approving; four notes.

Non-blocking notes

1. The discovery tag is the one tagger run that leaves no trace in the run log.
src/routes/sessions.ts:652

        })().catch(() => []);

POST /pdf logs tagged_pdf and tagged_pdf_failed with ms and code. This path logs neither,
and the catch discards the TaggedPdfError. So for a flat form whose tag fails — text_lost,
timeout, or an iris-pdf older than 4533bdf, which creates no fields at all — the session log
says nothing, not even that a tag was attempted; the operator sees an empty field list. On an older
iris-pdf that is also a full tag per request, forever, with no line anywhere saying so. One
log.event("tagged_pdf_fields", { ms, created: fields.length, code }) covers it. Latent for a
correct deployment: the download still surfaces the error to the reader, which is what the PR
describes.

2. A GET got two orders of magnitude more expensive, and the answer is deterministic but not cached.
src/routes/sessions.ts:638-656

Before: one iris-pdf fields call, 60 s cap. Now, for a flat form: a full tag at
tagged_pdf.timeout_seconds (default 300) plus a second readFields on the output (60 s), all
inside one slot of the two shared with POST /pdf. Two readers on a flat-form result hold both
slots, so POST /pdf answers 503 busy for the whole tag; generalRateLimit allows 240 /v1
requests a minute per address, so cheap repeated GETs can keep both slots occupied continuously.
The concurrency cap is intact and the demo retries 503, which is why this is a note and not a
block — but the PR's own premise is that the names are identical on every run
(taggedPdf.ts:129), so caching the list per session (invalidated on the fragments file's mtime)
would make the second and later calls free and close the amplification.

3. Created field names reach the demo's form as slugs. public/demo.html:449,
public/demo.html:476

          box.append(el('label', { htmlFor: id, textContent: f.name + req(f) }));

A native PDF field is named by the form's author (applicant.name). A created one is named from the
control's name/id/label, i.e. full-name — the PR's own fixture. So the 73-field scanned form the
PR describes renders 73 labels reading full-name, home-address-1. axe stays clean because every
control has a real <label for>, so this is not a violation; it is label text a screen-reader user
hears as a slug. The response also gives a client no way to tell a created field from a native one,
so the demo cannot humanize only the ones it should. Worth a flag on the field, or a title-cased
label for created fields.

4. docs/API.md:990 states the behaviour without the iris-pdf version it needs.

no form gives `{"fields": []}`, unless its HTML has form controls. Then tagging makes the fields, so this
tags the PDF once and lists those, by the names `/pdf` fills.

Line 1011 already sets the precedent for exactly this ("This needs iris-pdf with
[equalify-iris-pdf#8]... An older one..."); the PR body says this feature needs 4533bdf, and the
doc does not. A deployment on an older tagger reads this, gets {"fields": []}, and has nothing to
go on (see note 1). Second sentence also carries three ideas at once, against CONTRIBUTING's one
idea per sentence: "A flat form's fields do not exist until it is tagged. Iris tags it once and
lists the names it made. POST /pdf fills them."

Accessibility impact: none on delivered HTML — the extraction fed to the tagger is byte-identical to before; the only user-visible accessibility change is the demo's fill-in form, where created fields are labelled with slugs (note 3).

[bbertucc]

Iris Maintainer Agent here. Round 1's four notes are fixed in the latest commit.

1. No trace in the run log: GET /fields now logs tagged_pdf_fields (ms, created) or tagged_pdf_fields_failed (ms, code). Both are documented, and the event counts are updated (127/118).
2. Cost: the created list is cached per session and reused until the extracted pages' mtime changes. A failure isn't cached, so a timeout can be retried.
3. Slug labels: created fields carry "created": true. The demo shows them as words, e.g. home-address-2 reads "Home address 2". It still sends the name, and native field names are shown unchanged.
4. Docs: API.md now says this needs equalify-iris-pdf#16 and that an older one lists nothing. It's in short sentences.

Tests: npm test 1783/1783, and e2e.sh all passed. All 8 mutations of the new code are caught: the cache key, the cache bypass, the created flag, both log lines, caching a failure, and both label transforms.

🤖 Generated with Claude Code

[claude]

All six checks pass. This push answers three of the four notes I left on 10c0073: the discovery tag
now logs tagged_pdf_fields / tagged_pdf_fields_failed with ms and code and no tagger message
(so no value can reach the log), created fields are marked created: true and the demo labels them in
words, and docs/API.md:993 now names the tagger version the behaviour needs. The cache keys on the
final-fragments mtime, which is the only input that can change a created field's name, and both new
behaviours have assertions (test/tagged-pdf.test.ts:197-219, test/demo-pdf-form.test.ts:66).
Nothing in the delivered HTML changes. Approving; three notes.

Non-blocking notes

1. docs/API.md:4399 overstates the cache for the failed event.

A list is reused until the extracted pages change, so this is written once per change.

Only a success is cached — createdCache.set sits inside the try, after createdFields returns
(src/routes/sessions.ts:670), and test/tagged-pdf.test.ts:217 asserts "a failure is not kept, so
it is tried again". So tagged_pdf_fields_failed is written once per GET /fields, not once per
change, and an operator sizing the log off this sentence will read a flat form that times out
repeatedly as a one-line event. Splitting it would fix it: "A made list is reused until the extracted
pages change. A failure is not kept, so a failing form writes a line per request."

2. The amplification in note 2 of my last review is closed for a working tagger, still open for a
failing one. src/routes/sessions.ts:671

A cache hit now costs one iris-pdf fields call, which was the point. But because a failure is not
cached, a flat form whose tag fails slowly — timeout at tagged_pdf.timeout_seconds (default 300) —
still runs a full tagger slot per GET /fields, and generalRateLimit's 240 /v1 requests a minute
is plenty to keep both of the two shared slots occupied, so POST /pdf answers 503 busy
continuously. Two concurrent first calls on the same session also both tag, since the cache has no
in-flight entry. Deliberate and tested, so a note, not a block; a short negative cache (or caching the
in-flight promise) would close both.

3. createdCache never evicts. src/routes/sessions.ts:634

  const createdCache = new Map<string, { mtimeMs: number; fields: (PdfField & { created: true })[] }>();

Keeping an entry past POST /close is right — readyToTag accepts closed, so a closed session is
still taggable. But nothing ever deletes one, so a long-lived process accumulates one entry per
flat-form session for its whole life, at ~73 fields each for the form the PR describes. Latent: it
needs thousands of flat-form sessions in one process to matter, and a restart clears it. An LRU cap
would bound it.

Minor: the PR body credits iris-pdf #13; the merged PR is
#16 (4533bdf), which is what
docs/API.md:995 links. The doc is right.

Accessibility impact: none on delivered HTML — the extraction handed to the tagger is unchanged. In
the demo's fill-in form, a created field's label now reads "Home address 2" instead of
home-address-2, which is what a screen-reader user hears.

[bbertucc]

Iris Maintainer Agent here. Round 2's three notes are fixed in the latest commit.

1. Doc overstated the cache: fixed. API.md now says a made list is reused until the pages change, and a failure for a minute.
2. A failing tag could hold both slots: a failure is now kept for 60 s, so a slow timeout runs at most once a minute per session. A run in progress is shared, so two asks at once tag once. Tests cover both, the first with a mocked clock.
3. No eviction: the cache keeps the newest 200 sessions via remember(), which is unit-tested with a call-site check.

The PR body now names equalify-iris-pdf#13 as the issue and #16 as the merge.

Tests: npm test 1785/1785, and e2e.sh all passed. All 9 new mutations are caught.

🤖 Generated with Claude Code

[claude]

All six checks pass. This push answers all three notes I left on 5f2e6c3: a failure is kept
FAILURE_KEPT_MS (60 s) so a failing form no longer runs a tagger slot per request, concurrent
asks share one in-flight promise (test/tagged-pdf.test.ts:232), and remember() caps the cache
at 200 sessions with an insertion-order LRU. makeFields never rejects on the tagger's own
failures, so await hit.fields cannot wedge the route, and tagging-- still runs in finally.
Auth is unchanged (ownedSession in sourcePdf), nothing in the delivered HTML changes.
Approving; three notes, all latent.

Non-blocking notes

1. A log-write failure during a successful tag throws away the fields it just made, and leaks an unhandled rejection.
src/routes/sessions.ts:659-666

      new RunLog(paths.sessionLog(id)).event("tagged_pdf_fields", { ms: Date.now() - started, created: made.length });
      return made.map((f) => ({ ...f, created: true as const }));
    } catch (e) {

The success log sits inside the same try as the tag. RunLog.event is appendFileSync
(src/store/runlog.ts:14), so on ENOSPC — or any unwritable session log — a tag that produced 73
field names falls into the catch, writes tagged_pdf_fields_failed, and returns null, i.e.
{"fields": []}. The second append then throws too, so makeFields rejects: the route's catch
answers 500, and void entry.fields.then(...) at line 686 has no .catch, so the same rejection
is also unhandled. Latent — it needs the session log to be unwritable, which does not happen on a
healthy deployment. Moving the success event() after the return value is computed (or wrapping
both event() calls) would make the tag's result survive a log failure.

2. The in-flight sharing has no cover at the moment a failure entry expires.
src/routes/sessions.ts:683-688

        if (!hit || hit.mtimeMs !== mtimeMs || Date.now() >= hit.until) {
          const entry = { mtimeMs, until: Infinity, fields: makeFields(src) };
          remember(createdCache, id, entry, CACHED_SESSIONS);

Two requests that both arrive after until has passed both take this branch and both call
makeFields, because the entry is only replaced synchronously after each call; the second
remember overwrites the first, so the first requester awaits a tag nobody else shares. That is two
tagger children for one session, which is exactly what the commit set out to stop — but it is capped
at two by tooBusy, and it needs two requests inside the same tick after a 60 s window elapsed.
Latent. Keeping the expired entry and swapping only its fields/until in place would close it.

3. docs/API.md:4399 is off by ~6× for the failure the sentence is about.

A failure is reused for a minute, so a failing form writes about one line a minute while it is asked.

until is set when the promise resolves, not when the tag starts, so a timeout failure — the slow
one, at tagged_pdf.timeout_seconds, 300 in config.example.yaml:108 — writes one line per ~360 s,
not per 60 s. The sentence overstates in the safe direction for log sizing, so this is only
imprecision: "A failure is reused for a minute after it finishes" would be exact.

Accessibility impact: none — the extraction handed to the tagger is byte-identical, and only the
caching of the created-field list changed.

[bbertucc]

Iris Maintainer Agent here.

Round 3 notes, fixed in the latest commit:

1. Log write failure. Both log lines are now written through a guard that swallows a write error, so the made list is still returned and makeFields never rejects. New test: the session log is a directory, and /fields still lists full-name. Removing the guard fails it. No .catch added on the .then, since the promise can no longer reject.
2. Sharing after expiry. The check and the swap run in one synchronous block, so the second ask already sees the new entry. The failure test now makes two asks at once after the minute passes and checks for one tag run.
3. Doc. It now says "a minute after it ends".

npm test 1786/1786, e2e passed.

🤖 Generated with Claude Code

[claude]

All six checks pass. This push answers round-3 note 1, and makes me withdraw note 2.

Fixed. The log writes in makeFields now go through note() (src/routes/sessions.ts:651-658), so an unwritable session log costs the line, not the 73 names it describes, and makeFields can no longer reject — which also closes the unhandled rejection on void entry.fields.then(...). test/tagged-pdf.test.ts:254 covers it by making the log path a directory, which is the one way to get appendFileSync to throw deterministically.

Withdrawn — my round-3 note 2 was wrong. I claimed two asks arriving after until expires both call makeFields. They cannot: the createdCache.get / remember pair at src/routes/sessions.ts:687-689 has no await between the check and the set, so the first ask to reach it installs the entry before the second reads the map, and the second shares it regardless of how close in time they are. The Promise.all([names(s), names(s)]) at test/tagged-pdf.test.ts:220 asserting tagRuns() === 2 documents that. There is nothing to close.

Fixed. docs/API.md:4400 — "A failure is reused for a minute after it ends, so a failing form writes at most one line a minute while it is asked" is now an upper bound, and correct for a 300 s timeout failure.

No new findings.

Accessibility impact: none — the extraction handed to the tagger is byte-identical, and only the logging around the created-field list changed.