mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 00:26:26 +00:00
feat(layout): measure compiled PDF layout instead of eyeballing it (#378)
* feat(layout): measure compiled PDF layout instead of eyeballing it /apply Step 5b asks for layout properties and executes none of them: they are checked by reading the rendered page, which is exactly how they get missed. The failure that motivates this is silent under every existing check. A moderncv \cventry renders as a tabular, so an entry is one unbreakable block; when it does not fit in the space left, the whole entry moves to the next page and leaves a hole behind. The document still compiles, still reports the expected page count, and still passes tools/verify_pdf.py. Observed in the wild at 273pt, roughly 19 blank lines, mid-page, on a CV whose visual read looked fine. tools/verify_layout.py reports per page where the text starts and stops, bottom whitespace as a share of page height, and the largest gap between lines, then exits 1 on a hole over 100pt, a non-final page ending more than 25% early, body text colliding with the page-number footer, a final page more than 35% empty, or an entry header or section heading stranded at a page break. Page count is deliberately not checked here - verify_pdf.py --pages already does that, and two implementations of one rule drift. Geometry comes from Poppler pdftotext -bbox, already a dependency; a missing Poppler exits 2 with "skipped:" rather than failing the run. Tests build synthetic Page/Line geometry, so the suite needs neither Poppler nor a LaTeX toolchain and runs on the existing 3.10-3.14 matrix. * fix(layout): survive Windows encoding and a pdftotext without -bbox Review found two failures on the repo's primary platform: subprocess.run(..., text=True) decoded pdftotext's UTF-8 output with the Windows ANSI codepage and crashed on the stock cv/main_example.pdf. It now passes encoding="utf-8" with errors="replace" at the call site, matching the fix verify_pdf.py already carries from #369. Git for Windows ships an xpdf-based pdftotext that shadows Poppler in a default PATH and has no -bbox flag; it exited 99, the CalledProcessError escaped, and the run ended in exit 1 - indistinguishable from a real layout problem, which would send /apply chasing a phantom hole. That now routes to the existing "skipped:" exit 2 path with a message naming the likely cause, covered by three tests on the extractor-failure path. Also from review: - .claude/settings.json and security_guards.py gain the verify_layout.py permission entries. apply.md Step 5b runs the tool on every /apply, so without them every run prompts. - The docstring and CHANGELOG no longer call Poppler a dependency verify_pdf.py relies on. Since #369 verify_pdf prefers pypdf and Poppler is the fallback; word bboxes have no pypdf equivalent, so this is the one step that still wants it, and that is now what the text says. - Docstring and apply.md state that the thresholds are calibrated for the stock moderncv and cover.cls geometry, and that the shipped example CV fails the thin-final-page rule by design. - largest_gap documents that it measures top-to-top, so a tall line inflates the gap by its own height - over-detection, the safe direction. - The test module imports via tools.verify_layout like test_verify_pdf.py instead of sys.path.insert. * fix(verify_layout): quote only the first stderr line in the skip message (#378) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013fqqLgQSnwgWkv98twQhHi --------- Co-authored-by: Mads Lorentzen <madslorentzen_17@hotmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Yan Cheng (程彦)
co-authored by
Claude Fable 5.1
Mads Lorentzen
Claude Fable 5.1
Mads Lorentzen
parent
8c81edc330
commit
cbd8a991ab
@@ -0,0 +1,146 @@
|
||||
"""Offline tests for tools/verify_layout.py.
|
||||
|
||||
Every case is built from synthetic Page/Line geometry rather than a compiled
|
||||
PDF, so the suite needs neither Poppler nor a LaTeX toolchain - matching the
|
||||
repo's CI policy of keeping the Python tool tests self-contained.
|
||||
|
||||
The cases marked SILENT FAILURE are the ones that motivated the tool: each
|
||||
describes a document that compiles cleanly, reports the expected page count,
|
||||
and passes tools/verify_pdf.py, while the rendered page is visibly broken.
|
||||
"""
|
||||
|
||||
import io
|
||||
import subprocess
|
||||
import unittest
|
||||
from contextlib import redirect_stdout
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
from tools.verify_layout import Line, Page, find_orphans, main, parse_pdf, report
|
||||
|
||||
A4_HEIGHT = 842.0
|
||||
|
||||
|
||||
def line(top: float, left: float = 50.0, height: float = 10.0, text: str = "x") -> Line:
|
||||
return Line(top=top, bottom=top + height, left=left, height=height, text=text)
|
||||
|
||||
|
||||
class TestGapAndBottomSpace(unittest.TestCase):
|
||||
def setUp(self):
|
||||
# Three body lines, then a 322pt jump, then the page-number footer.
|
||||
self.holed = Page(
|
||||
A4_HEIGHT,
|
||||
[line(50), line(64), line(78), line(400), line(770, text="1/2")],
|
||||
)
|
||||
|
||||
def test_largest_gap_reports_size_and_position(self):
|
||||
"""SILENT FAILURE: the hole an ejected \\cventry leaves behind."""
|
||||
gap, y = self.holed.largest_gap()
|
||||
self.assertEqual((round(gap), round(y)), (322, 78))
|
||||
|
||||
def test_bottom_space_ignores_the_footer_band(self):
|
||||
"""Measured to the last body line (y410), not to the page number at y770."""
|
||||
self.assertEqual(round(self.holed.bottom_space), 432)
|
||||
|
||||
def test_page_with_no_body_lines_is_empty(self):
|
||||
self.assertTrue(Page(A4_HEIGHT, []).empty)
|
||||
|
||||
def test_report_flags_the_hole(self):
|
||||
with redirect_stdout(io.StringIO()): # report() prints its per-page measurements
|
||||
problems = report(Path("synthetic"), [self.holed])
|
||||
self.assertTrue(any("hole" in m for m in problems), problems)
|
||||
|
||||
|
||||
class TestFooterBand(unittest.TestCase):
|
||||
def test_single_line_in_band_is_just_the_page_number(self):
|
||||
self.assertFalse(Page(A4_HEIGHT, [line(50), line(800, text="2/2")]).footer_crowded)
|
||||
|
||||
def test_two_lines_in_band_means_body_text_spilled_in(self):
|
||||
"""SILENT FAILURE: \\enlargethispage pushing body text over the footer."""
|
||||
self.assertTrue(Page(A4_HEIGHT, [line(50), line(780), line(800)]).footer_crowded)
|
||||
|
||||
|
||||
class TestHeadingAndIndentDetection(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.page = Page(
|
||||
A4_HEIGHT,
|
||||
[
|
||||
line(50, height=16.0, text="Professional Experience"),
|
||||
line(80, left=50.0),
|
||||
line(94, left=70.0),
|
||||
],
|
||||
)
|
||||
|
||||
def test_taller_line_is_a_heading(self):
|
||||
self.assertTrue(self.page.is_heading(self.page.body[0]))
|
||||
self.assertFalse(self.page.is_heading(self.page.body[1]))
|
||||
|
||||
def test_left_edge_separates_bullets_from_headers(self):
|
||||
self.assertTrue(self.page.is_indented(self.page.body[2]))
|
||||
self.assertFalse(self.page.is_indented(self.page.body[1]))
|
||||
|
||||
|
||||
class TestOrphans(unittest.TestCase):
|
||||
def test_page_ending_on_a_section_heading(self):
|
||||
"""SILENT FAILURE: a heading stranded at the bottom, content overleaf."""
|
||||
p1 = Page(A4_HEIGHT, [line(50), line(64), line(700, height=16.0, text="Education")])
|
||||
p2 = Page(A4_HEIGHT, [line(60, text="Example University"), line(74, left=70.0)])
|
||||
self.assertTrue(any("ends on the section heading" in m for m in find_orphans([p1, p2])))
|
||||
|
||||
def test_entry_header_orphaned_from_its_bullets(self):
|
||||
"""SILENT FAILURE: the \\cventry title on one page, its bullets on the next."""
|
||||
q1 = Page(A4_HEIGHT, [line(50), line(700, left=50.0, text="Software Engineer")])
|
||||
q2 = Page(A4_HEIGHT, [line(60, left=70.0, text="- built the thing")])
|
||||
self.assertTrue(any("orphaned from its bullets" in m for m in find_orphans([q1, q2])))
|
||||
|
||||
def test_lone_list_marker_is_a_split_bullet_not_an_orphaned_header(self):
|
||||
"""moderncv gives the itemize marker its own bbox line: different defect, different fix."""
|
||||
m1 = Page(A4_HEIGHT, [line(50), line(700, left=50.0, text="●")])
|
||||
m2 = Page(A4_HEIGHT, [line(60, left=70.0, text="continued item text here")])
|
||||
self.assertTrue(any("lone list marker" in m for m in find_orphans([m1, m2])))
|
||||
|
||||
def test_clean_break_reports_nothing(self):
|
||||
r1 = Page(A4_HEIGHT, [line(50), line(700, left=50.0)])
|
||||
r2 = Page(A4_HEIGHT, [line(60, left=50.0)])
|
||||
self.assertEqual(find_orphans([r1, r2]), [])
|
||||
|
||||
def test_indent_is_judged_against_the_document_margin(self):
|
||||
"""A page that OPENS with bullets must not mistake their indent for its margin."""
|
||||
s1 = Page(A4_HEIGHT, [line(50, left=50.0), line(700, left=50.0, text="Data Analyst")])
|
||||
s2 = Page(A4_HEIGHT, [line(60, left=70.0, text="- first bullet")])
|
||||
self.assertTrue(any("orphaned from its bullets" in m for m in find_orphans([s1, s2])))
|
||||
|
||||
class TestExtractorFailure(unittest.TestCase):
|
||||
"""A broken extractor must not masquerade as a broken document.
|
||||
|
||||
Git for Windows ships an xpdf-based pdftotext with no -bbox flag; it shadows
|
||||
Poppler in a default PATH and exits 99. Reported as a layout problem it would
|
||||
send /apply chasing a phantom hole, so it has to land on the skip path.
|
||||
"""
|
||||
|
||||
def test_pdftotext_without_bbox_raises_a_skippable_error(self):
|
||||
failure = subprocess.CalledProcessError(99, "pdftotext", stderr="Error: unknown flag")
|
||||
with patch("tools.verify_layout.shutil.which", return_value="/usr/bin/pdftotext"), patch(
|
||||
"tools.verify_layout.subprocess.run", side_effect=failure
|
||||
):
|
||||
with self.assertRaisesRegex(RuntimeError, "bounding boxes"):
|
||||
parse_pdf(Path("cv/main_example.pdf"))
|
||||
|
||||
def test_missing_poppler_raises_a_skippable_error(self):
|
||||
with patch("tools.verify_layout.shutil.which", return_value=None):
|
||||
with self.assertRaisesRegex(RuntimeError, "not found"):
|
||||
parse_pdf(Path("cv/main_example.pdf"))
|
||||
|
||||
def test_extractor_failure_exits_2_not_1(self):
|
||||
"""Exit 1 means "your document is broken"; a dead extractor must never claim that."""
|
||||
with patch("tools.verify_layout.parse_pdf", side_effect=RuntimeError("no -bbox")), patch(
|
||||
"sys.argv", ["verify_layout.py", __file__]
|
||||
):
|
||||
err = io.StringIO()
|
||||
with redirect_stdout(io.StringIO()), patch("sys.stderr", err):
|
||||
self.assertEqual(main(), 2)
|
||||
self.assertIn("skipped:", err.getvalue())
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user