Drop --ocr, --verify, --verify-dpi, --partial and --retag - #10
Conversation
OCR is always auto, the checks always run, and a page with no way to place text is left untagged with a warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Checks all pass. The removed flags leave no dangling references (src/, test/, README.md, docs/ grepped clean), and the untagged-scan path still suppresses the PDF/UA-1 claim (src/tag.ts:109, !untagged), so nothing blocking.
Non-blocking notes
1. test/cli.test.ts:46 — the new test only hides Tesseract on some machines.
const r = spawnSync(process.execPath, [cli, ...tagArgs("mixed", out, "--report", report)], { encoding: "utf8", env: { PATH: dirname(process.execPath) } });dirname(process.execPath) hides tesseract only if node lives elsewhere. On this runner node is /opt/hostedtoolcache/node/24.21.0/x64/bin/node and tesseract /usr/bin/tesseract, so it passes; with node from apt/NodeSource (/usr/bin/node), or a tarball install next to a self-built /usr/local/bin/tesseract, the trimmed PATH still resolves tesseract. Reproduced by handing the CLI exactly the directory that holds tesseract:
$ PATH=/usr/bin node src/cli.ts tag --pdf test/fixtures/mixed.pdf \
--pages test/fixtures/mixed.pages.json --out /tmp/mixed-out.pdf --report /tmp/mixed-rep.json
exit=0
[ 'pdf-text', 'ocr' ] [ 'font_not_embedded', 'duplicate_text_layer' ]
Both assertions fail there: textSource is ocr, not none, and there is no no_text_positions warning. PATH: "" (or an empty temp dir) is environment-independent — node is started by absolute path and needs no PATH. This matters because with --ocr off gone this is now the only test of the untagged-scan path, and the README tells users to run npm test on install.
2. A scan without Tesseract now exits 0 and Iris's HTML for that page is dropped. src/tag.ts:154-157: every caller gets the old --partial behaviour and nothing can ask for "fully tagged or nothing". A pipeline that checks only the exit code ships a PDF whose scanned page has no structure, and the accessible HTML Iris produced for it is discarded. The warning plus the withheld PDF/UA-1 claim are the only signals, and README.md does not say that detecting this requires parsing --report. Worth one sentence there, since this is the PR's stated intent.
3. Tesseract present but failing is now unrecoverable. src/ocr/tesseract.ts:21 throws a plain Error on a non-zero exit (e.g. tesseract-ocr without tesseract-ocr-eng); cli.ts turns that into internal_error exit 3 for the whole document. --ocr off was the way to still tag the text pages of a mixed PDF; nothing replaces it.
4. Nits. test/cli.test.ts:40 now asserts only existsSync(twice) after an exit-0 assertion, and the structural retag coverage already lives in tag.test.ts. In README.md, (a scan, with Tesseract not installed; the page is left as it was; a blank page needs no HTML and is not warned) now hangs off three warning codes with each clause applying to a different one.
Accessibility impact: output accessibility is unchanged for text PDFs, but a scanned page on a box without Tesseract is now silently delivered untagged at exit 0 instead of refused, so callers must read the report to notice Iris's HTML was dropped.
…bly in the test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Checks all pass. Re-reviewing after bf45a0d.
Earlier notes: 1 (test hid Tesseract only on some machines) is fixed — test/cli.test.ts:49 now uses env: { PATH } with PATH: "", which is environment-independent. 3 (Tesseract present but failing was unrecoverable) is fixed — src/ocr/tesseract.ts:21 returns the reason instead of throwing, and the failing case is now covered by the second loop iteration of the new test. The README parenthetical nit is fixed.
Note 2 stands by design and is now documented (README.md:74): a scan on a box without Tesseract exits 0 with the page untagged and Iris's HTML for it discarded, with only the report and the withheld PDF/UA-1 claim as signals. I raised it once; it is the PR's stated intent, so I leave the "fully tagged or nothing" question to the maintainer.
Non-blocking notes
1. src/ocr/tesseract.ts:21 — the warning can read Tesseract failed: undefined.
if (run.status !== 0) return `Tesseract failed: ${run.stderr?.toString().trim() || run.error?.message}`;A tesseract that exits non-zero and writes nothing to stderr (a wrapper script, a build that only sets an exit code) gives stderr.trim() === "" and run.error === undefined, so the report's no_text_positions detail becomes "The page has no text layer and Tesseract failed: undefined; it was left untagged." Since the report is now the only way a pipeline learns the page was dropped, the detail is the diagnostic; a fallback like `exit ${run.status}` would keep it useful. The new test's fake binary writes to stderr, so it does not cover this.
2. test/cli.test.ts:37 — "a tagged PDF is retagged" now asserts only an exit code. The existsSync(twice) check went too, so the test would pass if the second run wrote nothing at that path. Structural retag coverage does live in document.test.ts, so this is only a weak test, not a gap.
3. --retag now exits 3 (src/cli.ts:19-23). main deliberately kept retag: { type: "boolean" }, // ignored, for older callers; removing it turns a currently-working Iris invocation into bad_arguments. The PR body says Iris must stop passing it (equalify-iris#505), so this is a merge-ordering matter, not a defect: land equalify-iris#505 first, or the sibling pipeline fails on every document.
Accessibility impact: unchanged for text PDFs; a scanned page whose Tesseract is missing or failing is now delivered untagged at exit 0 rather than refused, so callers must read the report to notice Iris's HTML was dropped — the PDF/UA-1 claim is still correctly withheld in that case.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Checks all pass. Re-review of 3fb4e0b (one-line change to src/ocr/tesseract.ts).
Earlier note 1 ("the warning can read Tesseract failed: undefined") is fixed — src/ocr/tesseract.ts:21 now falls back to `exit ${run.status}`. No remaining references to --ocr, --verify, --verify-dpi, --partial, --retag or --strict in src/, test/, docs/, README.md or .github/; the PDF/UA-1 claim is still withheld for an untagged page (src/tag.ts:113, !untagged).
Non-blocking notes
1. A scan untagged at exit 0 (raised on 3bf2de1 and bf45a0d). src/tag.ts:154-157: nothing can ask for "fully tagged or nothing", so a pipeline that checks only the exit code ships a PDF whose scanned page has no structure and discards Iris's HTML for it. It is now documented (README.md:74). I have raised this twice and it is the PR's stated intent — leaving the design question to the maintainer.
2. --retag now exits 3 (src/cli.ts). main deliberately kept retag: { type: "boolean" }, // ignored, for older callers; removing it turns a currently-working Iris invocation into bad_arguments for every document. Merge ordering, not a defect: land equalify-iris#505 first.
3. test/cli.test.ts:37 — "a tagged PDF is retagged" asserts only an exit code. The existsSync(twice) check went with --retag, so the test would pass if the second run wrote nothing at that path. Structural retag coverage does live in document.test.ts, so this is a weak test, not a gap.
4. src/ocr/tesseract.ts:21, continuing the fix in this commit. run.status is null when tesseract is killed by a signal (an OOM kill on a large 300-DPI pixmap is reachable with an attacker-sized scan), so a silent signal death reads Tesseract failed: exit null. run.signal would name it.
Accessibility impact: unchanged for text PDFs; a scanned page whose Tesseract is missing or failing is delivered untagged at exit 0 with the PDF/UA-1 claim correctly withheld, so callers must read the report to notice Iris's HTML was dropped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Checks all pass. Re-review of a6c7d47 (one line in src/ocr/tesseract.ts).
Earlier note 4 ("a silent signal death reads Tesseract failed: exit null") is fixed — src/ocr/tesseract.ts:21 now falls back to run.signal:
if (run.status !== 0) return `Tesseract failed: ${run.stderr?.toString().trim() || run.error?.message || run.signal || `exit ${run.status}`}`;An OOM-killed tesseract that wrote nothing now reads Tesseract failed: SIGKILL. Nothing else changed; the PDF/UA-1 claim is still withheld for an untagged page (src/tag.ts:109, !untagged), and no references to the dropped flags remain in src/, README.md, docs/ or .github/.
Non-blocking notes
Three carried forward unchanged, all raised before:
1. A scan untagged at exit 0 (src/tag.ts:154-157). Nothing can ask for "fully tagged or nothing", so a pipeline that checks only the exit code ships a PDF whose scanned page has no structure and discards Iris's HTML for it. Documented at README.md:74. Raised three times; it is the PR's stated intent, so this is the maintainer's call, not a defect.
2. --retag now exits 3 (src/cli.ts:16-19). main deliberately kept retag: { type: "boolean" }, // ignored, for older callers; dropping it turns a currently-working Iris invocation into bad_arguments for every document. Merge ordering: land equalify-iris#505 first.
3. test/cli.test.ts:37 — "a tagged PDF is retagged" asserts only an exit code. The existsSync(twice) check went with --retag, so the test would pass if the second run wrote nothing at that path. Structural retag coverage lives in document.test.ts, so this is a weak test, not a gap.
Accessibility impact: unchanged for text PDFs; a scanned page whose Tesseract is missing or failing is delivered untagged at exit 0 with the PDF/UA-1 claim correctly withheld, and the report's warning now names the signal when Tesseract is killed outright.
Fewer flags. OCR is always auto; the pixel and text checks always run at 150 DPI; a scan with Tesseract missing is left untagged with
no_text_positions(a warning, not a refusal).--retagis gone: tagging always replaces old tags, and the flag now exits 3. Iris must stop passing it (equalify-iris#505).🤖 Generated with Claude Code