From df566e78f1878877ae075daa60418d667be6714a Mon Sep 17 00:00:00 2001 From: oskar Date: Wed, 22 Jul 2026 19:13:21 +0200 Subject: [PATCH] fix(mail-body-ingest): html_to_text loses entire body on unclosed void elements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The naive LIFO tag stack pushed every start tag and popped on any end tag by position, not name. Real email HTML almost always writes void elements (,
, , ...) without a self-closing slash -- e.g. pops a "meta" frame for the literal , leaving skip_depth stuck at 1 for the rest of the document. Caught live during the faza-mailowa Etap A dry-run spot-check (plan §7, Krok 4 calibration step) against real archived mail: a genuine HTML-only promotional email extracted to '' entirely. Re-running the Etap A dry-run after the fix dropped body_empty from 3253/13300 (24.5%) to 291/13300 (2.2%) -- the bug was silently discarding real content from a meaningful slice of HTML-only mail. Fix: void elements are never pushed onto the stack (so they can never desync it); every other closing tag searches backward for its matching open tag and truncates the stack from there, which also self-heals other malformed nesting instead of only the void-tag case. 3 new regression tests reproduce the exact ... pattern, br/hr line breaks, and a stray unmatched closing tag. --- .../src/mail_body_ingest/ingest.py | 45 +++++++++++++------ jobs/mail-body-ingest/tests/test_ingest.py | 23 ++++++++++ 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/jobs/mail-body-ingest/src/mail_body_ingest/ingest.py b/jobs/mail-body-ingest/src/mail_body_ingest/ingest.py index e903adc..594a566 100644 --- a/jobs/mail-body-ingest/src/mail_body_ingest/ingest.py +++ b/jobs/mail-body-ingest/src/mail_body_ingest/ingest.py @@ -118,17 +118,26 @@ class _MailHTMLTextExtractor(HTMLParser): subtrees (Decision 2c — quoted reply chains in HTML mail), inserts newlines at block-level boundaries so `kb_mail.chunking.chunk_text`'s paragraph splitter has something to work with. - LIFO tag-stack assumption (no tag-name matching on close): correct for well-nested HTML, - which is what mail clients emit in practice; a truly malformed document may under/over-skip - at the margins -- acceptable for a best-effort heuristic with zero new dependencies. + Tag-name-matched stack (search backward on close, truncate from the match): real-world mail + HTML frequently writes void elements (``, `
`, ``, ...) without a self-closing + slash. A naive LIFO push/pop-on-any-endtag desyncs on these -- e.g. ` + ` pops a `meta` "frame" for the literal ``, leaving `skip_depth` stuck at 1 for + the rest of the document (confirmed live: a real HTML-only mail with a `` full of + `` tags extracted to '' entirely). Void elements are never pushed at all; every other + close searches backward for its matching open and discards anything opened-but-never-closed + after it, which also self-heals other malformed nesting instead of just the void-tag case. """ _SKIP_CONTENT_TAGS = {"style", "script", "head"} _BLOCK_TAGS = {"p", "div", "tr", "li", "h1", "h2", "h3", "h4", "h5", "h6", "br"} + _VOID_ELEMENTS = { + "area", "base", "br", "col", "embed", "hr", "img", "input", + "link", "meta", "param", "source", "track", "wbr", + } def __init__(self) -> None: super().__init__(convert_charrefs=True) - self._tag_stack: list[tuple[bool, bool]] = [] + self._tag_stack: list[tuple[str, bool, bool]] = [] self._skip_depth = 0 self._quote_depth = 0 self._parts: list[str] = [] @@ -137,6 +146,11 @@ class _MailHTMLTextExtractor(HTMLParser): return self._skip_depth == 0 and self._quote_depth == 0 def handle_starttag(self, tag: str, attrs: list) -> None: + if tag in self._VOID_ELEMENTS: + if self._visible() and tag in self._BLOCK_TAGS: + self._parts.append("\n") + return + was_visible = self._visible() starts_skip = tag in self._SKIP_CONTENT_TAGS starts_quote = tag == "blockquote" or ( @@ -146,20 +160,23 @@ class _MailHTMLTextExtractor(HTMLParser): self._skip_depth += 1 if starts_quote: self._quote_depth += 1 - self._tag_stack.append((starts_skip, starts_quote)) + self._tag_stack.append((tag, starts_skip, starts_quote)) if was_visible and tag in self._BLOCK_TAGS: self._parts.append("\n") def handle_endtag(self, tag: str) -> None: - if not self._tag_stack: - return - starts_skip, starts_quote = self._tag_stack.pop() - if starts_skip and self._skip_depth > 0: - self._skip_depth -= 1 - if starts_quote and self._quote_depth > 0: - self._quote_depth -= 1 - if self._visible() and tag in self._BLOCK_TAGS: - self._parts.append("\n") + for i in range(len(self._tag_stack) - 1, -1, -1): + if self._tag_stack[i][0] == tag: + _, starts_skip, starts_quote = self._tag_stack[i] + del self._tag_stack[i:] # also drops anything opened-but-never-closed after it + if starts_skip and self._skip_depth > 0: + self._skip_depth -= 1 + if starts_quote and self._quote_depth > 0: + self._quote_depth -= 1 + if self._visible() and tag in self._BLOCK_TAGS: + self._parts.append("\n") + return + # no matching open tag on the stack -- stray/unbalanced closing tag, ignore def handle_data(self, data: str) -> None: if self._visible(): diff --git a/jobs/mail-body-ingest/tests/test_ingest.py b/jobs/mail-body-ingest/tests/test_ingest.py index 48e4bba..60ce719 100644 --- a/jobs/mail-body-ingest/tests/test_ingest.py +++ b/jobs/mail-body-ingest/tests/test_ingest.py @@ -140,6 +140,29 @@ class TestHtmlToText: html = '
Still visible
' assert "Still visible" in html_to_text(html) + def test_unclosed_void_elements_in_head_do_not_blank_the_rest_of_the_document(self): + # Regression: a naive LIFO stack pops the wrong "frame" for the literal when + # tags inside are written without a self-closing slash (the overwhelming + # majority of real-world email HTML) -- confirmed live against an Etap A pilot mail + # where this bug silently blanked the entire body to ''. + html = ( + "" + '' + "promo" + "

Kod rabatowy dla Ciebie!

" + ) + assert "Kod rabatowy dla Ciebie!" in html_to_text(html) + + def test_br_and_hr_void_elements_still_produce_line_breaks(self): + html = "

line one
line two

line three

" + text = html_to_text(html) + assert "line one" in text and "line two" in text and "line three" in text + + def test_stray_unmatched_closing_tag_is_ignored_not_fatal(self): + html = "

hello

world

" + text = html_to_text(html) + assert "hello" in text and "world" in text + class TestIsNewsletter: def test_list_unsubscribe_present(self):