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):