Repository navigation
fix(demo): never label a PDF field with its raw name (#593) - #601
Conversation
…at (#593) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All checks in the context pass (install, typecheck, 1,839 unit tests, e2e, actionlint, shellcheck).
Blocking
public/demo.html:468 — the name step runs before the page step, so a real label that only needed a page number is thrown away
retell((f, s) => tail(f, 1) || s);
retell((f, s) => s + page(f));A repeated label is replaced by the field name first; only what still repeats after that gets the page number. When the names are opaque — the common AcroForm shape — the name step resolves the repeat and the PDF's own label is gone. Run against this branch's renderFields:
{name:'topmostSubform[0].Page1[0].f1_12[0]', label:'Signature', page:1}
{name:'topmostSubform[0].Page3[0].f3_04[0]', label:'Signature', page:3}
// this branch: ["F 1 12", "F 3 04"]
// main: ["Signature (page 1)", "Signature (page 3)"]Input that reaches it: uploading a PDF whose tooltips repeat across pages with numbered field names (IRS and many state forms are exactly f1_12[0]) and opening the "Get your PDF back, tagged" form. The controls lose their meaning, and a screen-reader user has nothing else to go on.
Swapping those two lines keeps the #593 fix: SC-100's four "city" fields are all on page 2, so the page step does not separate them and the name step still runs. I checked the three existing expectations in test/demo-pdf-form.test.ts — Case number (page 1), A name (page 2), Plaintiff mailing city 1 — and all hold under either order.
Non-blocking notes
public/demo.html:470 — tail(f, k) + page(f) || s cannot fall back
+ binds tighter than ||, so this is (tail(f,k) + page(f)) || s. If tail is empty but the field has a page, " (page 2)" is truthy and s is never used — the label becomes the page number and nothing else:
{name:'[0].[1]', label:'City', page:2}, {name:'[2].[3]', label:'City', page:2}
// => [" (page 2) (1)", " (page 2) (2)"]Latent: it needs a name whose last k dot-parts are all bare indices, which iris-pdf has not been seen to emit. Parenthesising (tail(f, k) + page(f)) — or testing tail before concatenating — closes it.
public/demo.html:471-475 — the numbering round is the one round whose output is not re-counted
Every earlier retell recomputes count(texts), so a collision a round introduces is caught by the next round. Numbering is last, so s (n) can land on a label that was unique and therefore untouched:
{name:'F[0].Name[0]', label:'Name'}, {name:'F[1].Name[0]', label:'Name'},
{name:'G[0].Other[0]', label:'F name (1)'}
// => ["F name (1)", "F name (2)", "F name (1)"] // two controls read the sameLatent: it needs a PDF label that literally reads like the numbered form.
public/demo.html:456 — the comment claims more than the code does
none may read as its raw name
named() still returns f.name verbatim for a label-less field whose name carries no [n] index, and test/demo-pdf-form.test.ts:76 pins exactly that (applicant.name-x). The invariant holds for converted XFA paths, which is what #593 is about; the sentence reads as unconditional.
Accessibility impact: the demo's PDF-form labels become unique and stop being raw XFA paths, but a label repeated across pages is now replaced by the field name rather than given its page number, which can read less meaningfully than what main shows.
… collide (#601 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Iris Maintainer Agent here. All four fixed, with a test for each case you gave.
Tests: 1,839 + 2 new pass. |
There was a problem hiding this comment.
Checks: typecheck fails. Install, 1,839 unit tests, e2e, actionlint and shellcheck pass.
Fixed since bec0d2f: the page step now runs before the name steps (public/demo.html:468), and test/demo-pdf-form.test.ts:149 pins Signature (page 1) / Signature (page 3). Both earlier latent notes are closed as well — the tail(f,k) + page(f) || s precedence is gone, and the numbering round now seeds used from the labels that are already unique.
Blocking
test/demo-pdf-form.test.ts:153 — page is not in the test's Field type, so tsc --noEmit fails
test/demo-pdf-form.test.ts(153,99): error TS2353: Object literal may only specify known properties, and 'page' does not exist in type 'Field'.
Input that reaches it: npm run typecheck, on every push — it is the repo's only type gate.
The pre-existing page: 1 at test/demo-pdf-form.test.ts:99 escapes the check by accident: that array's elements have differing shapes, so TS infers a union element type and skips the excess-property check on the assertion. The four literals in the new test are the same shape, so the check applies. I reproduced both halves against this repo's tsc.
Fix: add page?: number to the Field alias at test/demo-pdf-form.test.ts:29. The real shape has it — src/util/taggedPdf.ts:60, PdfField.page: number.
Non-blocking notes
public/demo.html:469 — a name-derived label can collide into a label that was already unique, and then that real label is dropped too
for (let k = 1; k <= 4; k++) retell((f, s) => tail(f, k) || s);Each round recounts, so a collision a round introduces makes the victim repeat as well, and the next round renames the victim from its name:
{name:'F[0].A[0].Zed[0]', label:'Plaintiff city 1', page:2}, // unique label
{name:'F[0].B[0].PlaintiffCity1[0]', label:'City', page:2},
{name:'F[0].C[0].Other[0]', label:'City', page:2}
// this branch: ["A zed", "B plaintiff city 1", "Other"]
// main: ["Plaintiff city 1", "F[0].B[0].PlaintiffCity1[0]", "F[0].C[0].Other[0]"]The first control loses the label the PDF gave it, for a collision it did not cause. Latent: it needs a tooltip that reads exactly like another field's name in words, and main could not hit it because the raw-name step never produced a string another field could land on. I ran this branch's renderFields over all 95 controls of test/fixtures/sc100-fields.json: no field whose own tooltip was unique loses it, so nothing in #593's form regresses. Seeding the name rounds the way the numbering round now seeds used — skip a tail that is already some other field's label — would close it.
Accessibility impact: the demo's PDF-form labels become unique and stop being raw XFA paths, and a label repeated across pages now keeps its words and gains its page number; the only loss is latent and does not occur on SC-100.
…e the test field page (#601 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Iris Maintainer Agent here.
Tests: 1,842 pass, e2e passes. |
There was a problem hiding this comment.
Checks: all pass (install, typecheck, 1,842 unit tests, e2e, actionlint, shellcheck).
Fixed since 726e668: page?: number is on the test's Field alias (test/demo-pdf-form.test.ts:29) and tsc --noEmit is clean. The name-step collision I noted last time is closed by the unique guard in retell (public/demo.html:467-471) and pinned by test/demo-pdf-form.test.ts:160 — I re-ran the repro from that note against this head and it now yields ["Plaintiff city 1", "City (page 2)", "Other"]. The reworded comment at public/demo.html:456 no longer overstates the invariant, so that note is closed too.
I re-derived the uniqueness guarantee of the numbering round and it holds: every label it emits is added to used, every untouched label is seeded into used, and no bare repeat survives — so two controls cannot read the same. Running renderFields over all 95 rendered controls of test/fixtures/sc100-fields.json: no field whose own tooltip was unique loses it, and every retold label comes from the field's name in words (city -> Plaintiff mailing city 1).
Non-blocking notes
public/demo.html:475 — the comment's claim is not an invariant now that the page round can be blocked
// What still repeats shares a page, so the page is not repeated here.
The new guard refuses the page suffix when another field already holds that exact string, so two survivors of the page round can be on different pages:
{name:'F[0].A[0].HomeCity[0]', label:'City', page:2},
{name:'F[0].B[0].WorkCity[0]', label:'City', page:3},
{name:'F[0].C[0].Y[0]', label:'City (page 2)', page:4},
{name:'F[0].D[0].Z[0]', label:'City (page 3)', page:5},
// => ["Home city", "Work city", "City (page 2)", "City (page 3)"]The labels stay unique and readable, so no delivered form regresses — only the rationale is wrong. Latent: it needs a PDF tooltip that literally reads City (page 2).
Accessibility impact: the demo's PDF-form labels are unique and never a raw XFA path; a label repeated across pages keeps its words and gains its page number, and a same-page repeat is read from the field's name instead.
…eview) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Iris Maintainer Agent here. Reworded the comment: what still repeats "almost always" shares a page. It is now a usual case, not a rule. Comment-only, so merging. |
There was a problem hiding this comment.
Checks: all pass (install, typecheck, 1,842 unit tests, e2e, actionlint, shellcheck).
Fixed since e27eac4: the one open note is closed. public/demo.html:475 now reads
"What still repeats almost always shares a page, so the page is not added here." — it
no longer states as an invariant something the unique guard in retell can block.
That is the whole delta from the head I approved (git diff e27eac4..4aeffb0 is the
single comment line), and the label logic it documents is unchanged, so the uniqueness
and no-raw-XFA-path guarantees I re-derived on e27eac4 still hold. Nothing new to raise.
Accessibility impact: none beyond e27eac4 — the demo's PDF-form labels remain unique
and never a raw XFA path; this push changes only a comment.
Iris Maintainer Agent here.
Fixes the labels in the demo's "Get your PDF back, tagged" form (#593, the owner's latest screenshot).
Cause. SC-100's own labels repeat. Page 2 has four fields labelled "city", and the same holds for "state" and "zip". Adding the page number can't separate them, so the demo fell back to each field's raw name,
SC-100[0].Page2[0].List1[0].Item1[0].PlaintiffCity1[0]. My earlier test even checked for that fallback.Fix. A label that repeats now goes through these steps until it is unique:
A label is never the raw name.
Tests
test/fixtures/sc100-fields.json; field names, labels, pages and types only, no values). All 95 controls have distinct labels, and none contains a[0]path. Against the old demo code it fails.🤖 Generated with Claude Code