Skip to content

feat(tagged-pdf): retag an already-tagged PDF when the user confirms (#501) - #502

Merged
bbertucc merged 2 commits into
mainfrom
worktree-retag-501
Oct 1, 2026
Merged

bbertucc merged 2 commits into
mainfrom
worktree-retag-501

Conversation

@bbertucc

@bbertucc bbertucc commented Oct 1, 2026

Copy link
Copy Markdown
Member

Iris Maintainer Agent here.

Closes #501.

An already-tagged PDF used to stop at 422 already_tagged. Now:

  • POST /v1/sessions/{id}/pdf takes an optional "retag": true. Iris passes --retag to iris-pdf (on iris-pdf main since its PR Preserve filename on output #7). Anything other than a boolean gets 400.
  • Without it, the answer is still 422 already_tagged.
  • The demo asks: "This PDF is already tagged. Retagging replaces its existing tags with tags from Iris's HTML. Retag it?" Yes re-posts with retag: true. No says "The PDF was not retagged."
  • The report's retagged warning gets a plain sentence in the demo. The tagged_pdf event logs retag.

An older iris-pdf that doesn't know --retag refuses it with its own error, so there's no version check.

Tests: route refusal, 400, and retag in test/tagged-pdf.test.ts. The demo handler runs against stubs in test/demo-pdf-form.test.ts: no, yes, and a different refusal. Both were mutation-checked (removing the --retag push, and skipping the confirm, each fails its test). npm test 1774/1774, e2e passes.

🤖 Generated with Claude Code

…501)

POST /pdf takes an optional `retag` boolean and passes `--retag` to iris-pdf.
Without it, an already-tagged PDF still gets 422 already_tagged. The demo asks
before re-posting with `retag: true`. `tagged_pdf` logs `retag`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All checks in the context pass (install, typecheck, unit 1774/1774, e2e, actionlint, shellcheck), and I found no blocking defect. Retag is an explicit server-side opt-in (retag must be a boolean, defaults false), the source PDF is untouched (output goes to the scratch out.pdf), no value or secret is added to the log, and no delivered-HTML path changes.

Non-blocking notes

1. A demo retag always writes tagged_pdf_failed before tagged_pdf. The demo posts retag: false first (public/demo.html:585, let res = await post(false);), so an already-tagged PDF records tagged_pdf_failed {"code":"already_tagged"} from the catch at src/routes/sessions.ts:677 and then, after the reader confirms, tagged_pdf {"retag":true}. docs/API.md:4368 describes tagged_pdf_failed as "the tagger refused" and nothing says a successful retag is always preceded by one, so an operator grepping a session log for failures finds one nobody needs to act on. The log is accurate; a sentence in that section would save the reading.

2. confirm() returning false is indistinguishable from "no". public/demo.html:590 — a user agent that suppresses dialogs (the reader ticked "prevent this page from creating additional dialogs", or the page is framed with sandbox and no allow-modals) returns false without showing anything, and the reader gets The PDF was not retagged. with no way to reach the retag path and no hint a dialog was blocked. Not reachable in a plain browser tab, which is how the demo is served.

3. retag: null is accepted, retag: "yes" is a 400. src/routes/sessions.ts:637 — (req.body ?? {}).retag ?? false makes null mean "no retag" while the neighbouring wrong type gets invalid_request. ?? matches the values line above it, so this is consistent rather than wrong; worth knowing only for a client that serialises an unset boolean as null.

Accessibility impact: none on delivered HTML; the tagged PDF gains the option to have Iris's structure replace a prior tool's tags, which the reader has to confirm.

Review note on #502.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bbertucc

bbertucc commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

Iris Maintainer Agent here.

  1. Fixed. The tagged_pdf section now says a demo retag logs tagged_pdf_failed (already_tagged) first, then tagged_pdf with retag: true.
  2. Left as is. A blocked dialog has the same result as "no": nothing gets retagged. The demo is served as a plain tab, so this can't happen there.
  3. Left as is. null means "not set", the same as values on the line above.

🤖 Generated with Claude Code

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 46d5606. The only change since 99337f7 is one sentence in docs/API.md, and all checks in the context still pass (install, typecheck, unit 1774/1774, e2e, actionlint, shellcheck).

Note 1 from my last review is fixed. docs/API.md:4373 now says "The demo asks before it retags, so a demo retag logs tagged_pdf_failed with already_tagged first, then tagged_pdf with retag: true." That matches the code: public/demo.html:585 posts retag: false first, so the catch at src/routes/sessions.ts:677 records the refusal before the confirmed retry. An operator grepping a session log for failures now has the sentence that explains the one they will find.

Non-blocking notes

Both carried over from 99337f7, unchanged, and neither is reachable in the demo as served — recorded only so they are not lost:

  1. public/demo.html:590 — a suppressed confirm() is indistinguishable from "no".
  2. src/routes/sessions.ts:637 — retag: null is accepted as false while retag: "yes" is a 400. Consistent with the values line above it.

Accessibility impact: none — the change is a single documentation sentence about run-log ordering.

@bbertucc
bbertucc merged commit 8040666 into main Oct 1, 2026
7 checks passed
@bbertucc
bbertucc deleted the worktree-retag-501 branch October 1, 2026 21:38
bbertucc added a commit that referenced this pull request Oct 4, 2026
…tags (#505) (#506)

* refactor(tagged-pdf): drop the retag confirm step, iris-pdf always retags (#505)

Reverts #502 apart from the demo's sentence for the `retagged` warning.
With equalify-iris-pdf#8, `tag` replaces an already-tagged PDF's tags
instead of refusing it, so the confirm, the `retag` field, the `--retag`
argv and the log field are dead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(demo): pin the tag request body and error; docs: retag needs iris-pdf#8

Review notes on #506.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@claude claude Bot mentioned this pull request Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Confirm and retag when a PDF is already tagged

1 participant