mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-11-10 17:07:40 +00:00
* fix(verify-layout): walk PATH candidates for pdftotext supporting -bbox (#655) Git for Windows prepends C:\Program Files\Git\mingw64\bin to PATH under Git Bash (which Claude Code's Bash tool uses on Windows). That directory ships an xpdf pdftotext 4.00 without -bbox support. Previously, verify_layout.py resolved whichever pdftotext shutil.which found first, causing the layout check to exit 2 with skipped: on Windows even when a working Poppler build (e.g. from MiKTeX, Chocolatey, or Scoop) was installed further down PATH. Walk all candidates returned by shutil.which("pdftotext", path=entry) for each entry in os.get_exec_path(), de-duplicated. Probe each candidate's -h output for -bbox support, and call subprocess.run with the full path of the first working binary instead of the bare command name. If no candidate supports -bbox, keep today's skip message on failure. Tests added to TestExtractorFailure in tests/test_verify_layout.py: - First candidate lacks -bbox and second has it (second candidate's full path is run) - No candidate has -bbox (skip path is kept) - PATH candidate de-duplication (avoids redundant probes) Fixes #655 * perf(verify-layout): cache resolved pdftotext binary at module level * changelog: place the #668 entry under [Unreleased] Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Mads Lorentzen <madslorentzen_17@hotmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4 files changed
+187
-8
No files matched your search
@@ -268,7 +268,7 @@ The layout script reports, per page, where the text starts and stops, bottom whi
|
|||||||
|
|
||||||
The hole check is the one a visual read misses. A moderncv `\cventry` renders as a `tabular`, so it is an **unbreakable block**: when it does not fit in the space left, the whole entry jumps to the next page and leaves a hole behind, while the document still compiles and still reports the right page count. Fix it by shortening the entry that follows the hole, not by stretching the page.
|
The hole check is the one a visual read misses. A moderncv `\cventry` renders as a `tabular`, so it is an **unbreakable block**: when it does not fit in the space left, the whole entry jumps to the next page and leaves a hole behind, while the document still compiles and still reports the right page count. Fix it by shortening the entry that follows the hole, not by stretching the page.
|
||||||
|
|
||||||
If Poppler is missing, or the `pdftotext` first in PATH is the xpdf build Git for Windows ships (no `-bbox`), the script exits 2 with `skipped:` — note the degraded mode in the Step 6 report and rely on the visual inspection alone. Exit 2 is never a layout verdict.
|
If Poppler is missing, or no `pdftotext` on PATH supports `-bbox` (Git for Windows ships an xpdf build without it), the script exits 2 with `skipped:` - note the degraded mode in the Step 6 report and rely on the visual inspection alone. Exit 2 is never a layout verdict.
|
||||||
|
|
||||||
The thresholds are calibrated for the stock moderncv and `cover.cls` geometry; a template registered via `/add-template` may report a phantom hole above a footer the 90pt band does not cover.
|
The thresholds are calibrated for the stock moderncv and `cover.cls` geometry; a template registered via `/add-template` may report a phantom hole above a footer the 90pt band does not cover.
|
||||||
|
|
||||||
|
|||||||
@@ -172,6 +172,13 @@ per-file diff commands.
|
|||||||
requires every shipped heading to appear in order, so an added section passes and a renamed,
|
requires every shipped heading to appear in order, so an added section passes and a renamed,
|
||||||
dropped or reordered one still fails; upstream keeps the exact comparison.
|
dropped or reordered one still fails; upstream keeps the exact comparison.
|
||||||
|
|
||||||
|
- **`verify_layout.py` uses a `pdftotext` that supports `-bbox` wherever it sits on PATH** (#668,
|
||||||
|
fixes #655, `tools/verify_layout.py`, `tests/test_verify_layout.py`, `.claude/commands/apply.md`) -
|
||||||
|
under Git Bash, which Claude Code uses on Windows, Git's xpdf `pdftotext` 4.00 (no `-bbox`) sits
|
||||||
|
ahead of MiKTeX's Poppler, so the layout check skipped on the standard Windows setup even with
|
||||||
|
Poppler installed. It now probes each distinct `pdftotext` on PATH with `-h`, runs the first that
|
||||||
|
lists `-bbox` by its full path, and keeps the `skipped:` exit 2 for when none does.
|
||||||
|
|
||||||
## [1.7.3] - 2026-10-19
|
## [1.7.3] - 2026-10-19
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
@@ -274,6 +274,14 @@ class TestExtractorFailure(unittest.TestCase):
|
|||||||
send /apply chasing a phantom hole, so it has to land on the skip path.
|
send /apply chasing a phantom hole, so it has to land on the skip path.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
super().setUp()
|
||||||
|
verify_layout.find_pdftotext.cache_clear()
|
||||||
|
|
||||||
|
def tearDown(self):
|
||||||
|
super().tearDown()
|
||||||
|
verify_layout.find_pdftotext.cache_clear()
|
||||||
|
|
||||||
def test_pdftotext_without_bbox_raises_a_skippable_error(self):
|
def test_pdftotext_without_bbox_raises_a_skippable_error(self):
|
||||||
failure = subprocess.CalledProcessError(99, "pdftotext", stderr="Error: unknown flag")
|
failure = subprocess.CalledProcessError(99, "pdftotext", stderr="Error: unknown flag")
|
||||||
with patch("tools.verify_layout.shutil.which", return_value="/usr/bin/pdftotext"), patch(
|
with patch("tools.verify_layout.shutil.which", return_value="/usr/bin/pdftotext"), patch(
|
||||||
@@ -282,6 +290,127 @@ class TestExtractorFailure(unittest.TestCase):
|
|||||||
with self.assertRaisesRegex(RuntimeError, "bounding boxes"):
|
with self.assertRaisesRegex(RuntimeError, "bounding boxes"):
|
||||||
parse_pdf(Path("cv/main_example.pdf"))
|
parse_pdf(Path("cv/main_example.pdf"))
|
||||||
|
|
||||||
|
def test_first_candidate_lacks_bbox_and_second_has_it(self):
|
||||||
|
"""When the first pdftotext candidate lacks -bbox (e.g. Git for Windows's xpdf)
|
||||||
|
and a subsequent candidate has it (e.g. Poppler), the tool walks PATH, probes
|
||||||
|
with -h, and runs the working candidate with its full path (#655).
|
||||||
|
"""
|
||||||
|
def fake_which(cmd, path=None):
|
||||||
|
if path == "/git/bin":
|
||||||
|
return "/git/bin/pdftotext"
|
||||||
|
if path == "/miktex/bin":
|
||||||
|
return "/miktex/bin/pdftotext"
|
||||||
|
return None
|
||||||
|
|
||||||
|
def fake_run(cmd, *args, **kwargs):
|
||||||
|
binary = cmd[0]
|
||||||
|
if "-h" in cmd:
|
||||||
|
if binary == "/git/bin/pdftotext":
|
||||||
|
return subprocess.CompletedProcess(cmd, 99, stdout="", stderr="Usage: pdftotext ...")
|
||||||
|
if binary == "/miktex/bin/pdftotext":
|
||||||
|
return subprocess.CompletedProcess(
|
||||||
|
cmd, 99, stdout="", stderr="Usage: pdftotext ...\n -bbox : output bounding box"
|
||||||
|
)
|
||||||
|
if "-bbox" in cmd:
|
||||||
|
return subprocess.CompletedProcess(
|
||||||
|
cmd, 0, stdout='<page width="100" height="100"></page>', stderr=""
|
||||||
|
)
|
||||||
|
raise AssertionError(f"unexpected command: {cmd}")
|
||||||
|
|
||||||
|
with patch("tools.verify_layout.os.get_exec_path", return_value=["/git/bin", "/miktex/bin"]), patch(
|
||||||
|
"tools.verify_layout.shutil.which", side_effect=fake_which
|
||||||
|
), patch("tools.verify_layout.subprocess.run", side_effect=fake_run) as mock_run:
|
||||||
|
pages = parse_pdf(Path("cv/main_example.pdf"))
|
||||||
|
self.assertEqual(len(pages), 1)
|
||||||
|
bbox_calls = [call for call in mock_run.call_args_list if "-bbox" in call[0][0]]
|
||||||
|
self.assertEqual(len(bbox_calls), 1)
|
||||||
|
self.assertEqual(bbox_calls[0][0][0][0], "/miktex/bin/pdftotext")
|
||||||
|
|
||||||
|
def test_no_candidate_has_bbox_keeps_skip_path(self):
|
||||||
|
"""When no candidate on PATH supports -bbox, the check skips with today's message (#655)."""
|
||||||
|
def fake_which(cmd, path=None):
|
||||||
|
if path == "/git/bin":
|
||||||
|
return "/git/bin/pdftotext"
|
||||||
|
if path == "/other/bin":
|
||||||
|
return "/other/bin/pdftotext"
|
||||||
|
return None
|
||||||
|
|
||||||
|
failure = subprocess.CalledProcessError(99, "/git/bin/pdftotext", stderr="Error: unknown flag")
|
||||||
|
|
||||||
|
def fake_run(cmd, *args, **kwargs):
|
||||||
|
if "-h" in cmd:
|
||||||
|
return subprocess.CompletedProcess(cmd, 99, stdout="", stderr="Usage: pdftotext (xpdf, no bbox)")
|
||||||
|
if "-bbox" in cmd:
|
||||||
|
raise failure
|
||||||
|
raise AssertionError(f"unexpected command: {cmd}")
|
||||||
|
|
||||||
|
with patch("tools.verify_layout.os.get_exec_path", return_value=["/git/bin", "/other/bin"]), patch(
|
||||||
|
"tools.verify_layout.shutil.which", side_effect=fake_which
|
||||||
|
), patch("tools.verify_layout.subprocess.run", side_effect=fake_run):
|
||||||
|
with self.assertRaisesRegex(RuntimeError, "bounding boxes") as ctx:
|
||||||
|
parse_pdf(Path("cv/main_example.pdf"))
|
||||||
|
message = str(ctx.exception)
|
||||||
|
self.assertIn("xpdf", message)
|
||||||
|
self.assertIn("Poppler aborted", message)
|
||||||
|
|
||||||
|
def test_find_pdftotext_deduplicates_path_candidates(self):
|
||||||
|
"""Duplicate entries or multiple PATH directories resolving to the same binary
|
||||||
|
are probed only once (#655).
|
||||||
|
"""
|
||||||
|
def fake_which(cmd, path=None):
|
||||||
|
return "/same/pdftotext"
|
||||||
|
|
||||||
|
h_calls = []
|
||||||
|
|
||||||
|
def fake_run(cmd, *args, **kwargs):
|
||||||
|
if "-h" in cmd:
|
||||||
|
h_calls.append(cmd[0])
|
||||||
|
return subprocess.CompletedProcess(
|
||||||
|
cmd, 99, stdout="", stderr="Usage: pdftotext ...\n -bbox : output bounding box"
|
||||||
|
)
|
||||||
|
raise AssertionError(f"unexpected command: {cmd}")
|
||||||
|
|
||||||
|
with patch("tools.verify_layout.os.get_exec_path", return_value=["/bin1", "/bin2", "/bin3"]), patch(
|
||||||
|
"tools.verify_layout.shutil.which", side_effect=fake_which
|
||||||
|
), patch("tools.verify_layout.subprocess.run", side_effect=fake_run):
|
||||||
|
selected = verify_layout.find_pdftotext()
|
||||||
|
self.assertEqual(selected, "/same/pdftotext")
|
||||||
|
self.assertEqual(h_calls, ["/same/pdftotext"])
|
||||||
|
|
||||||
|
def test_find_pdftotext_caches_resolution(self):
|
||||||
|
"""Resolved binary is cached at module level so repeated parse_pdf calls
|
||||||
|
avoid re-probing PATH candidates with subprocesses (#655).
|
||||||
|
"""
|
||||||
|
def fake_which(cmd, path=None):
|
||||||
|
return "/miktex/bin/pdftotext"
|
||||||
|
|
||||||
|
h_calls = []
|
||||||
|
bbox_calls = []
|
||||||
|
|
||||||
|
def fake_run(cmd, *args, **kwargs):
|
||||||
|
binary = cmd[0]
|
||||||
|
if "-h" in cmd:
|
||||||
|
h_calls.append(binary)
|
||||||
|
return subprocess.CompletedProcess(
|
||||||
|
cmd, 0, stdout="Usage: pdftotext ...\n -bbox : output bounding box", stderr=""
|
||||||
|
)
|
||||||
|
if "-bbox" in cmd:
|
||||||
|
bbox_calls.append(binary)
|
||||||
|
return subprocess.CompletedProcess(
|
||||||
|
cmd, 0, stdout='<page width="100" height="100"></page>', stderr=""
|
||||||
|
)
|
||||||
|
raise AssertionError(f"unexpected command: {cmd}")
|
||||||
|
|
||||||
|
with patch("tools.verify_layout.os.get_exec_path", return_value=["/miktex/bin"]), patch(
|
||||||
|
"tools.verify_layout.shutil.which", side_effect=fake_which
|
||||||
|
), patch("tools.verify_layout.subprocess.run", side_effect=fake_run):
|
||||||
|
pages1 = parse_pdf(Path("cv/main_example.pdf"))
|
||||||
|
pages2 = parse_pdf(Path("cv/main_example.pdf"))
|
||||||
|
self.assertEqual(len(pages1), 1)
|
||||||
|
self.assertEqual(len(pages2), 1)
|
||||||
|
self.assertEqual(h_calls, ["/miktex/bin/pdftotext"])
|
||||||
|
self.assertEqual(bbox_calls, ["/miktex/bin/pdftotext", "/miktex/bin/pdftotext"])
|
||||||
|
|
||||||
def test_poppler_abort_on_empty_info_string_names_that_cause_too(self):
|
def test_poppler_abort_on_empty_info_string_names_that_cause_too(self):
|
||||||
"""Poppler 26.0x before 26.05 aborts -bbox on an empty Info-dict string, e.g.
|
"""Poppler 26.0x before 26.05 aborts -bbox on an empty Info-dict string, e.g.
|
||||||
the empty /Title hyperref writes when pdftitle is unset (#451). That crash is
|
the empty /Title hyperref writes when pdftitle is unset (#451). That crash is
|
||||||
|
|||||||
+50
-7
@@ -27,9 +27,9 @@ Geometry comes from Poppler word bounding boxes (`pdftotext -bbox`). Poppler is
|
|||||||
repo-wide - since #369 `verify_pdf.py` prefers pypdf and falls back to Poppler - but word
|
repo-wide - since #369 `verify_pdf.py` prefers pypdf and falls back to Poppler - but word
|
||||||
bounding boxes have no pypdf equivalent, so this is the one step that still wants it.
|
bounding boxes have no pypdf equivalent, so this is the one step that still wants it.
|
||||||
Without it, or with a broken extractor, the check reports `skipped:` and exits 2 rather
|
Without it, or with a broken extractor, the check reports `skipped:` and exits 2 rather
|
||||||
than inventing a layout failure. A broken extractor has two distinct causes: an xpdf-based
|
than inventing a layout failure. A broken extractor has two distinct causes: no `pdftotext`
|
||||||
`pdftotext` (Git for Windows ships one ahead of Poppler in PATH) has no `-bbox` flag at
|
on PATH has the `-bbox` flag (Git for Windows ships an xpdf build without it, and the check
|
||||||
all, and Poppler 26.0x before 26.05 aborts `-bbox`/`-bbox-layout`/`-htmlmeta` on a PDF
|
skips past it to any Poppler further down PATH), and Poppler 26.0x before 26.05 aborts `-bbox`/`-bbox-layout`/`-htmlmeta` on a PDF
|
||||||
whose Info dictionary carries an empty string in any field - which `hyperref` writes for
|
whose Info dictionary carries an empty string in any field - which `hyperref` writes for
|
||||||
every field it does not set, so any `lualatex`/`pdflatex` document built with `hyperref`
|
every field it does not set, so any `lualatex`/`pdflatex` document built with `hyperref`
|
||||||
and no `\\hypersetup{pdftitle=...}` triggers a real Poppler crashing on a legal PDF (#451).
|
and no `\\hypersetup{pdftitle=...}` triggers a real Poppler crashing on a legal PDF (#451).
|
||||||
@@ -59,6 +59,8 @@ suite needs neither Poppler nor a compiled PDF.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import argparse
|
import argparse
|
||||||
|
import functools
|
||||||
|
import os
|
||||||
import re
|
import re
|
||||||
import shutil
|
import shutil
|
||||||
import subprocess
|
import subprocess
|
||||||
@@ -183,12 +185,53 @@ class Page:
|
|||||||
return max((tops[i + 1] - tops[i], tops[i]) for i in range(len(tops) - 1))
|
return max((tops[i + 1] - tops[i], tops[i]) for i in range(len(tops) - 1))
|
||||||
|
|
||||||
|
|
||||||
|
@functools.cache
|
||||||
|
def find_pdftotext() -> str | None:
|
||||||
|
"""Find a pdftotext binary that supports Poppler's -bbox flag.
|
||||||
|
|
||||||
|
Git for Windows prepends Git's mingw64\\bin to PATH under Git Bash (used by
|
||||||
|
Claude Code), shipping an xpdf pdftotext without -bbox. If a working Poppler
|
||||||
|
is installed further down PATH (e.g. MiKTeX, Chocolatey, Scoop), probe each
|
||||||
|
candidate with -h and pick the first that advertises -bbox (#655). Falls back
|
||||||
|
to the first candidate if none advertises -bbox, so the caller keeps today's
|
||||||
|
skip message on failure. Returns None if no pdftotext is found at all.
|
||||||
|
|
||||||
|
Cached at module level to avoid re-probing candidates with subprocesses on
|
||||||
|
every parse_pdf call within a process.
|
||||||
|
"""
|
||||||
|
candidates: list[str] = []
|
||||||
|
seen: set[str] = set()
|
||||||
|
for entry in os.get_exec_path():
|
||||||
|
candidate = shutil.which("pdftotext", path=entry)
|
||||||
|
if candidate and candidate not in seen:
|
||||||
|
seen.add(candidate)
|
||||||
|
candidates.append(candidate)
|
||||||
|
if not candidates:
|
||||||
|
return None
|
||||||
|
for candidate in candidates:
|
||||||
|
try:
|
||||||
|
res = subprocess.run(
|
||||||
|
[candidate, "-h"],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
encoding="utf-8",
|
||||||
|
errors="replace",
|
||||||
|
)
|
||||||
|
output = (res.stdout or "") + (res.stderr or "")
|
||||||
|
if "-bbox" in output:
|
||||||
|
return candidate
|
||||||
|
except (subprocess.SubprocessError, OSError):
|
||||||
|
continue
|
||||||
|
return candidates[0]
|
||||||
|
|
||||||
|
|
||||||
def parse_pdf(path: Path) -> list[Page]:
|
def parse_pdf(path: Path) -> list[Page]:
|
||||||
if not shutil.which("pdftotext"):
|
binary = find_pdftotext()
|
||||||
|
if not binary:
|
||||||
raise RuntimeError("pdftotext (Poppler) not found; install poppler-utils")
|
raise RuntimeError("pdftotext (Poppler) not found; install poppler-utils")
|
||||||
try:
|
try:
|
||||||
out = subprocess.run(
|
out = subprocess.run(
|
||||||
["pdftotext", "-bbox", "-enc", "UTF-8", str(path), "-"],
|
[binary, "-bbox", "-enc", "UTF-8", str(path), "-"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
# pdftotext emits UTF-8; without this Windows decodes it as cp1252 and
|
# pdftotext emits UTF-8; without this Windows decodes it as cp1252 and
|
||||||
@@ -210,8 +253,8 @@ def parse_pdf(path: Path) -> list[Page]:
|
|||||||
detail = stderr_lines[0] if stderr_lines else f"exit {exc.returncode}"
|
detail = stderr_lines[0] if stderr_lines else f"exit {exc.returncode}"
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
f"pdftotext could not produce bounding boxes for {path} ({detail}); "
|
f"pdftotext could not produce bounding boxes for {path} ({detail}); "
|
||||||
"either the pdftotext first in PATH is an xpdf build with no -bbox flag "
|
"either no pdftotext on PATH has the -bbox flag (Git for Windows ships an "
|
||||||
"(Git for Windows ships one ahead of Poppler), or Poppler aborted on this "
|
"xpdf build without it), or Poppler aborted on this "
|
||||||
"document - Poppler 26.0x before 26.05 aborts on a PDF whose Info "
|
"document - Poppler 26.0x before 26.05 aborts on a PDF whose Info "
|
||||||
"dictionary carries an empty string, as hyperref writes when pdftitle is unset"
|
"dictionary carries an empty string, as hyperref writes when pdftitle is unset"
|
||||||
) from exc
|
) from exc
|
||||||
|
|||||||
Reference in new issue
Block a user