From e355eafe27b95b7fc767d2a09c503d13fb7343cb Mon Sep 17 00:00:00 2001 From: Yang <116134061+yang2632@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:53:33 +0200 Subject: [PATCH] fix(verify-layout): find an orphaned entry header by where the text starts (#508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit find_orphans() compared each line's left edge with the document margin: an un-indented last line followed by an indented first line was an entry header orphaned from its bullets. A list marker moves a line's left edge without moving its text, so the rule misread two templates: * one that merges each bullet's marker into its first line (#481): a bullet wrapping across the break was reported as an orphaned header, and a real orphaned header followed by such bullets was not reported; * the stock CV, when its outer marker extracts as a line of its own: a [Job Title] header left at the foot of a page, bullets overleaf, was not reported (its text at x70.4 counts as indented), and a one-line list item whose outer marker is set 1.3pt below it - so the marker sorts last on the page - was reported as a split bullet although the whole item is on that page. Following discussion #481, the rule now reads text x: * parse_pdf records where a line's text starts after leading LIST_MARKERS words (Line.text_left, defaulted). The set is the agreed Unicode bullets without the dashes (`• ‣ ▪ ● ·`): a bullet's second line may open with an em dash, and the stock Honors item's second line opens with an en dash. Other glyphs are text, so a line that merges the stock outer marker (`○` on some setups, an emoji on others) keeps the marker's x, as the left-edge rule read it; * find_orphans compares the text x of a page's last printed line with that of the next page's first printed line, and reports an orphaned header when the latter is more than INDENT_PT further in. Bbox lines whose tops are within SAME_PRINTED_LINE_PT (3pt) are one printed line: pdftotext's rounded-yMin buckets split a line where a bold word sits 0.2pt off, or where a marker sits 1.3pt below its text. A line whose text starts more than SET_IN_LIMIT_PT (60pt) past the margin is set right or centred (a running header) and is skipped as the opener; * a page ending on a marker with no text beside it keeps the lone-marker report, with its condition and message unchanged. Stock cv/main_example.tex, \vspace* of 0-600pt in 8pt steps before four entries, 304 lualatex builds (and the same 304 with xelatex): this adds 15 reports, each a [Job Title] header at the foot of a page with its bullets overleaf; the other 289 builds give byte-identical output. The reported. A two-line outer item whose marker sorts last and that wraps across the break is no longer reported as a split bullet: it is read like the #481 wrapped bullet, which is expected clean. Tests: TestOrphansReadTextX (13; 7 fixes and the wrapped two-line item fail on master's logic, 5 pin behaviour master already had) and TestTextStart (4). On Python 3.10-3.14 `python -W error::SyntaxWarning -m unittest discover -s tests -t .` passes 520 tests (master 503); lint_skills.py, check_framework_version.py and security_guards.py OK. --- CHANGELOG.md | 19 +++++ tests/test_verify_layout.py | 157 +++++++++++++++++++++++++++++++++++- tools/verify_layout.py | 105 +++++++++++++++++++++--- 3 files changed, 267 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c755084..0ba8aed0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,25 @@ per-file diff commands. ### Fixed +- **`verify_layout.py` finds an orphaned entry header by where the text starts** + (`tools/verify_layout.py`, `tests/test_verify_layout.py`) - the orphan rule compared line + left edges against the document margin, and a list marker moves a line's left edge without + moving its text. A template that merges each bullet's marker into its first line (#481) got + a bullet wrapping across the page break reported as an orphaned header, and a real orphaned + header followed by such bullets not reported at all. On the stock CV, when the outer marker + extracts as a line of its own, a `[Job Title]` header left at the foot of a page with its + bullets overleaf was not reported (its text at x70.4 counted as indented), and a list item + whose outer marker is set 1.3pt below its line was reported as a split bullet. The rule now + compares where the text of the page's last printed line starts - after any leading bullet + glyph from `• ‣ ▪ ● ·` (ASCII, dashes and other glyphs are text) - with where the text of + the next page's first printed line starts, and reports an orphaned header when the latter + is set further in. Bbox lines whose tops are within 3pt count as one printed line, and a + line whose text starts more than 60pt past the margin (a running header, a right-set date) + is not where the next page resumes. A lone marker with no text beside it keeps the + split-bullet report. On 304 builds of the stock CV (`\vspace*` 0-600pt before four + entries) this adds 15 reports, each a `[Job Title]` header at the foot of a page with its + bullets overleaf, and the other 289 builds give the same output as before. + - **One posting on a portal without a numeric id no longer gets a second key when its title is re-listed with different casing** (#501, `tools/job_key.py`, `tests/test_job_key.py`) - the key hashed a slugified title alongside the URL whenever diff --git a/tests/test_verify_layout.py b/tests/test_verify_layout.py index f9948720..07f9977b 100644 --- a/tests/test_verify_layout.py +++ b/tests/test_verify_layout.py @@ -17,7 +17,7 @@ from pathlib import Path from unittest.mock import patch from tools import verify_layout -from tools.verify_layout import Line, Page, find_orphans, main, parse_pdf, report +from tools.verify_layout import Line, Page, find_orphans, main, parse_pdf, report, text_start A4_HEIGHT = 842.0 @@ -111,6 +111,161 @@ class TestOrphans(unittest.TestCase): 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]))) + +def at(top, left, text, text_left=None, bottom=None): + """A bbox line; text_left is where its text starts after a merged list marker.""" + bottom = top + 14.0 if bottom is None else bottom + return Line(top=top, bottom=bottom, left=left, height=bottom - top, text=text, + text_left=text_left) + + +class TestOrphansReadTextX(unittest.TestCase): + """The orphan rule compares where the TEXT starts, not where the line starts. + + Geometry from the #481 repro: entry headers at x59.5; a bullet's marker merged into + its first line at x58.9, its text at x71.6-72.1, continuations at x72.1. From the + stock CV: header text at x70.4, an inner bullet's marker in its own line at x71.5 + with the text at x81.3, and an outer marker at x59.0 that extracts either as a line + of its own or merged into the header line, depending on the font setup. + """ + + def test_bullet_wrapping_across_the_break_is_not_an_orphaned_header(self): + p1 = Page(A4_HEIGHT, [ + at(50, 59.5, "Plant Grower Example Garden 1981-1990"), + at(700, 58.9, "\u2022 Replanted the long border with plants raised", 72.1), + ]) + p2 = Page(A4_HEIGHT, [at(60, 72.1, "from seed, lifting the old clumps")]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_bullet_whose_text_opens_with_a_symbol_is_still_a_wrapped_bullet(self): + p1 = Page(A4_HEIGHT, [ + at(50, 59.5, "Plant Grower Example Garden 1981-1990"), + at(700, 58.9, "\u2022 (in winter) relaid the gravel paths along", 72.1), + ]) + p2 = Page(A4_HEIGHT, [at(60, 72.1, "the old orchard wall and round the pond")]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_continuation_opening_with_a_dash_is_still_a_wrapped_bullet(self): + """Dashes are not markers: an em dash can open a bullet's second line.""" + p1 = Page(A4_HEIGHT, [at(700, 58.9, "\u2022 Laid a new cobbled path, so", 72.1)]) + p2 = Page(A4_HEIGHT, [at(60, 72.1, "\u2014 that rain ran off into the borders")]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_header_level_with_merged_bullets_is_an_orphan(self): + """The bullet line starts at x58.9, left of the header; its text is further in.""" + p1 = Page(A4_HEIGHT, [at(700, 59.5, "Kitchen Gardener 1975-1980")]) + p2 = Page(A4_HEIGHT, [at(60, 58.9, "\u2022 Worked in the kitchen garden", 71.6)]) + self.assertTrue(any("orphaned from its bullets" in m for m in find_orphans([p1, p2]))) + + def test_stock_header_whose_outer_marker_is_its_own_line(self): + """The header's text at x70.4 counts as indented, so the left-edge rule missed it.""" + p1 = Page(A4_HEIGHT, [ + at(323, 59.5, "Professional Experience"), + at(695.9, 59.0, "\u25cb", bottom=707.6), + at(700.5, 70.4, "[Job Title] [YYYY-YYYY]"), + ]) + p2 = Page(A4_HEIGHT, [ + at(62.4, 71.5, "-", bottom=79.5), + at(64.4, 81.3, "[Achievement or responsibility 1]"), + ]) + self.assertTrue(any("orphaned from its bullets" in m for m in find_orphans([p1, p2]))) + + def test_stock_header_with_its_outer_marker_merged_in(self): + """`○` is not a LIST_MARKERS glyph, so the header keeps x59.0, as the left-edge rule read it.""" + p1 = Page(A4_HEIGHT, [at(700, 59.0, "\u25cb [Job Title] [YYYY-YYYY]")]) + p2 = Page(A4_HEIGHT, [at(62.4, 71.5, "-", bottom=79.5), at(64.4, 81.3, "[Achievement 1]")]) + self.assertTrue(any("orphaned from its bullets" in m for m in find_orphans([p1, p2]))) + + def test_stock_degree_and_description_stay_reported_with_the_marker_merged(self): + """Both texts at x70.4, but the merged `○` keeps the header at x59.0.""" + p1 = Page(A4_HEIGHT, [at(700, 59.0, "\u25cb [Degree] in [Field] [YYYY-YYYY]")]) + p2 = Page(A4_HEIGHT, [at(60, 70.4, "[Brief description or key topics.]")]) + self.assertTrue(any("orphaned from its bullets" in m for m in find_orphans([p1, p2]))) + + def test_outer_marker_set_below_a_whole_item_is_not_a_split_bullet(self): + """Stock CV: the outer marker sits 1.3pt below its one-line item and sorts last. + + The left-edge rule read it as a lone marker whose item was split, although the + whole item is on p1 and p2 opens with the next item. + """ + p1 = Page(A4_HEIGHT, [ + at(50, 59.5, "Core Competencies", bottom=70.5), + at(728.81, 70.4, "[Skill Category 4]: [Domain expertise, methods.]", bottom=743.4), + at(730.09, 59.0, "\U0001f7e4", bottom=741.8), + ]) + p2 = Page(A4_HEIGHT, [ + at(66.8, 70.4, "[Skill Category 5]: [Tools, platforms, software.]", bottom=81.4), + at(68.1, 59.0, "\U0001f7e4", bottom=79.8), + ]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_two_line_item_wrapping_across_the_break_is_read_like_a_wrapped_bullet(self): + """A behaviour change, not a fix: master called this a split bullet, and it is split. + + The item's first line is on p1 with its marker 1.3pt below it, and its second line + opens p2 at the item's text x - the #481 wrapped bullet, which is expected clean. + """ + p1 = Page(A4_HEIGHT, [ + at(696.4, 59.5, "Honors and Awards", bottom=716.9), + at(722.13, 70.4, "[Award Name] for the long-running community orchard", bottom=736.7), + at(723.41, 59.0, "\U0001f7e4", bottom=735.09), + ]) + p2 = Page(A4_HEIGHT, [at(66.8, 69.4, "\u2013 [Event/Organization] ([Year]).")]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_a_printed_line_split_by_font_is_read_as_one(self): + """pdftotext's rounded-yMin buckets split a printed line when a bold word sits 0.2pt off.""" + p1 = Page(A4_HEIGHT, [ + at(721.44, 125.6, "operations for the data team and later modules.]"), + at(721.64, 70.4, "Kubernetes Terraform"), + ]) + p2 = Page(A4_HEIGHT, [at(66.8, 70.4, "[Skill Category 3]: [Specific skills]")]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_stock_bullet_wrapping_across_the_break(self): + p1 = Page(A4_HEIGHT, [at(700, 81.3, "[Achievement or responsibility 1 - be")]) + p2 = Page(A4_HEIGHT, [at(60, 81.3, "specific, use numbers where possible]")]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_a_running_header_on_the_next_page_is_not_where_the_body_resumes(self): + """A right-aligned header at x459.8 is not text 'set further in' than a bullet.""" + p1 = Page(A4_HEIGHT, [ + at(50, 59.5, "Professional Experience"), + at(700, 81.3, "[Achievement or responsibility 2]"), + ]) + p2 = Page(A4_HEIGHT, [ + at(27, 459.8, "[First] [Last] \u2013 CV"), + at(62, 70.4, "[Company] [City, Country]"), + ]) + self.assertEqual(find_orphans([p1, p2]), []) + + def test_an_orphan_behind_a_running_header_is_still_found(self): + p1 = Page(A4_HEIGHT, [at(50, 59.5, "Professional Experience"), at(700, 70.4, "[Job Title]")]) + p2 = Page(A4_HEIGHT, [ + at(27, 459.8, "[First] [Last] \u2013 CV"), + at(62.4, 71.5, "-", bottom=79.5), + at(64.4, 81.3, "[Achievement or responsibility 1]"), + ]) + self.assertTrue(any("orphaned from its bullets" in m for m in find_orphans([p1, p2]))) + + +class TestTextStart(unittest.TestCase): + """parse_pdf's text_left, from (xMin, yMin, yMax, text) words sorted left to right.""" + + def test_a_leading_list_marker_is_set_aside(self): + self.assertEqual(text_start([(58.9, 0, 1, "\u2022"), (72.1, 0, 1, "Laid")]), 72.1) + + def test_no_marker_means_no_separate_text_start(self): + self.assertIsNone(text_start([(59.5, 0, 1, "Kitchen"), (94.9, 0, 1, "Gardener")])) + + def test_ascii_dashes_and_other_glyphs_are_text(self): + for glyph in ("-", "*", "\u2013", "\u2014", "\u25cb"): + with self.subTest(glyph=glyph): + self.assertIsNone(text_start([(69.4, 0, 1, glyph), (77.7, 0, 1, "[Event]")])) + + def test_a_bare_marker_has_no_text(self): + self.assertIsNone(text_start([(59.0, 0, 1, "\u2022")])) + class TestExtractorFailure(unittest.TestCase): """A broken extractor must not masquerade as a broken document. diff --git a/tools/verify_layout.py b/tools/verify_layout.py index d43e097c..7bd0eb4c 100644 --- a/tools/verify_layout.py +++ b/tools/verify_layout.py @@ -34,8 +34,9 @@ whose Info dictionary carries an empty string in any field - which `hyperref` wr 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). Line height serves -as a font-size proxy to spot section headings; left edge (xMin) separates bullet lines -from entry headers. +as a font-size proxy to spot section headings. An orphaned entry header is found by where +each line's TEXT starts, after any list marker merged into it, not by its left edge (see +`find_orphans`). The thresholds below are calibrated for the stock moderncv (`cv/`) and cover.cls (`cover_letters/`) geometry. A template registered via `/add-template` may need them @@ -84,6 +85,24 @@ FOOTER_BAND_PT = 90.0 # continuation, not an entry header or a section heading. INDENT_PT = 8.0 +# Glyphs that label a list item when a template merges them into the item's first line. +# Unicode only: ASCII `-` and `*`, and the dashes, also open real text (the continuation +# of a bullet may start with an em dash), so a line opening with one is read as text. +LIST_MARKERS = frozenset("\u2022\u2023\u25aa\u25cf\u00b7") + +# Lines whose tops are this close are parts of one printed line. pdftotext -bbox output +# is grouped by rounded yMin, which splits a printed line wherever words in another +# font sit a fraction of a point off and a .5 boundary falls between them (a bold word +# 0.2pt off; a list marker set 1.3pt below its item's first line on the stock CV). +# Consecutive printed lines are ~12pt apart. +SAME_PRINTED_LINE_PT = 3.0 + +# A list sets its text in by a label's width: 11-25pt past the margin on the stock CV +# and cover letter. A line whose text starts further in than this is set right or +# centred (a running header, a date on a line of its own), not where a page's body +# resumes. +SET_IN_LIMIT_PT = 60.0 + # A line this much taller than the body median is a section heading. HEADING_HEIGHT_RATIO = 1.25 @@ -91,6 +110,7 @@ PAGE_RE = re.compile(r'(.*?)', r WORD_RE = re.compile( r'([^<]*)' ) +TEXT_RE = re.compile(r"\w") @dataclass @@ -100,6 +120,13 @@ class Line: left: float height: float text: str + # Where the text starts once leading LIST_MARKERS words are set aside; None when the + # line has no such marker, so a Line built from `left` alone behaves as before. + text_left: float | None = None + + @property + def text_x(self) -> float: + return self.left if self.text_left is None else self.text_left class Page: @@ -201,6 +228,7 @@ def parse_pdf(path: Path) -> list[Page]: left=min(w[0] for w in words), height=max(w[2] - w[1] for w in words), text=" ".join(w[3] for w in sorted(words)), + text_left=text_start(sorted(words)), ) for words in buckets.values() ] @@ -208,13 +236,44 @@ def parse_pdf(path: Path) -> list[Page]: return pages +def text_start(words: list[tuple[float, float, float, str]]) -> float | None: + """xMin of the first word after leading LIST_MARKERS words, if there were any.""" + lead = 0 + while lead < len(words) and words[lead][3] and set(words[lead][3]) <= LIST_MARKERS: + lead += 1 + return words[lead][0] if 0 < lead < len(words) else None + + +def printed_text_x(lines: list[Line]) -> float | None: + """Where the text of one printed line starts, from the bbox lines that make it up.""" + xs = [l.text_x for l in lines if TEXT_RE.search(l.text)] + return min(xs) if xs else None + + def find_orphans(pages: list[Page]) -> list[str]: """A page ending on an entry header or section heading whose content resumes overleaf. Two shapes, both documented failures: * the last body line of a page is a section heading (stranded heading) - * the last body lines are un-indented (an entry header) while the next page opens - with indented bullet lines, i.e. the entry was split across the break + * the next page's text starts further in than the text of the page's last printed + line (an entry header), i.e. the entry was split across the break + + The second compares where the TEXT starts, not where the line starts. A template may + merge a list marker into the line it labels, which moves the line's left edge but not + its text: in the #481 repro a bullet's first line starts at x58.9 (the marker), level + with the entry headers at x59.5, while its text and its continuation start at x72.1. + Read by left edge, a bullet wrapping across the break looked like a header followed by + indented text, and a real header followed by such a bullet looked level. On the stock + CV the outer marker may extract as a line of its own; then the header's text (x70.4) + counted as indented and a header orphaned from its bullets (x81.3) went unreported. + + Only LIST_MARKERS glyphs are set aside. Where a marker is not among them - the stock + CV's outer marker extracts as `○` on some font setups and as an emoji on others - the + line keeps the marker's x, which is exactly what the left-edge rule read. That is how + a stock `[Degree]` line and its description (both text at x70.4) stay reported when + that marker merges into the header line (header x59.0), as they were. When the marker + extracts as its own line, the two are level and nothing tells them from a wrapped + line without font information, so neither this rule nor the left-edge one reports it. """ problems = [] # Indentation must be judged against the document's left margin, not each page's own @@ -240,23 +299,43 @@ def find_orphans(pages: list[Page]) -> list[str]: f"on p{i + 2}. Shorten the entry that follows it, or let the heading and its " "first entry move to the next page together" ) - elif not indented(last) and indented(first): + continue + # The last printed line: the bbox lines within SAME_PRINTED_LINE_PT of the last one. + tail = [l for l in here.body if l.top >= last.top - SAME_PRINTED_LINE_PT] + last_x = printed_text_x(tail) + if last_x is None: # moderncv puts an itemize marker in its own bbox line at the list's left # edge, so a list item split across the break looks like an un-indented # header followed by indented text. Different defect, different fix. - if not re.search(r"\w", last.text): + if not indented(last) and indented(first): problems.append( f"p{i + 1} ends on a lone list marker whose text continues on p{i + 2} " f"({first.text.strip()[:60]!r}): a bullet is split across the page break. " "Shorten the preceding content so the whole item fits on one page" ) - else: - problems.append( - f"p{i + 1} ends on the un-indented line {last.text.strip()[:60]!r} while " - f"p{i + 2} opens with the indented line {first.text.strip()[:60]!r}: an entry " - "header is orphaned from its bullets. Add \\needspace before that " - "\\cventry, or shorten it" - ) + continue + # The next page's first printed line with text, skipping a marker in its own line + # and a line set right or centred. + opener = next( + ( + l + for l in nxt.body + if TEXT_RE.search(l.text) and l.text_x <= doc_left + SET_IN_LIMIT_PT + ), + None, + ) + if opener is None: + continue + head = [l for l in nxt.body if abs(l.top - opener.top) <= SAME_PRINTED_LINE_PT] + next_x = printed_text_x(head) + if next_x is not None and next_x > last_x + INDENT_PT: + shown = max((l for l in tail if TEXT_RE.search(l.text)), key=lambda l: len(l.text)) + problems.append( + f"p{i + 1} ends on {shown.text.strip()[:60]!r} while the text on p{i + 2} " + f"starts further in ({opener.text.strip()[:60]!r}): an entry header is " + "orphaned from its bullets. Add \\needspace before that \\cventry, " + "or shorten it" + ) return problems