mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-08-13 04:13:35 +08:00
### Summary `format_document_soup` tracks "am I inside a table" and "am I inside a link" with sticky flags that are meant to be reset by `elif e.name == "/table"` and `elif e.name == "/a"`. BeautifulSoup's `.descendants` only yields opening tags — a `Tag` named `/table` or `/a` never exists — so both branches are dead code and neither flag is ever cleared. Everything after the first `<table>` on a page is therefore formatted as if it were still table content: paragraphs lose their newline, list items lose their `- ` marker, headings lose their break, and the text is glued onto the last table cell. Under `HTML_BASED_CONNECTOR_TRANSFORM_LINKS_STRATEGY=markdown` the same bug leaks a link's `href` into everything that follows it, including whole subsequent paragraphs. The Confluence connector (`confluence_connector.py:948`) goes through this path. Real output for a Confluence-shaped page (heading, intro, spec table, then the body) via the public `parse_html_page_basic`: **Before** ``` prod us-east-1 Rollback procedure If the canary fails, run the rollback script immediately. Drain the load balancer Revert the deployment Escalate to the on-call rota if the rollback stalls. Do not skip the post-mortem. ``` **After** ``` prod us-east-1 Rollback procedure If the canary fails, run the rollback script immediately. - Drain the load balancer - Revert the deployment Escalate to [the on-call rota](http://oncall.example.com) if the rollback stalls. Do not skip the post-mortem. ``` Every heading, paragraph and list marker after the table is lost, and the whole body is indexed as one run-on line hanging off a table cell. ### Fix Derive both scopes from each element's **ancestors** instead of from flags that nothing can clear, and drop the two dead branches plus the two that become redundant. The scopes are resolved in one up-front pass into `id`-keyed maps (`table_scope`, `href_scope`) and looked up in O(1) per element. Probing per element with `find_parent` instead is O(depth) each, which measured 12–13× slower on table-heavy pages and up to 103× on deeply nested markup; the map version costs a depth-independent 1.13–1.35× over `main`. Numbers and method are in the round-2 comment below. This also changes one adjacent behaviour worth calling out explicitly: a link **inside** a table cell now renders as markdown, where before it rendered as plain text. That previous behaviour was not by design — it only held when no link preceded the table. With a link before the table, `main` stamps the stale href onto every cell: ``` main: '[pre](http://STALE.com)\n\t[cellA](http://STALE.com)\t[cellB](http://STALE.com)' branch: '[pre](http://STALE.com)\n\tcellA\tcellB' ``` Those cells are not links. Both symptoms are the same sticky-state bug, so they are fixed together rather than left half-done. ### Testing `test/unit_test/data_source/test_html_utils.py` is new — `format_document_soup` had no test coverage. 11 tests: 8 fail on `main` and pass on this branch, 3 are controls that pass on both (the table itself still separates rows and cells, anchor text is still linkified, the default `strip` strategy still strips). Representative failures on `main`: ``` assert '\nAfter' in 'Before\n\tA\tB After' assert '\n- item1' in 'Before\n\tA\tB item1 item2' assert 'see [link](http://x.com) [ after](http://x.com)' == 'see [link](http://x.com) after' assert '[next paragraph]' not in '[link](http://x.com)\n[next paragraph](http://x.com)' ``` Reverting each clause of the fix independently keeps the anchors honest: reverting only the table clause fails exactly the 4 table tests and leaves the link tests green; reverting only the link clause fails exactly the 3 link tests and leaves the table tests green. (`test_link_inside_a_table_cell_is_linkified` needs both clauses broken to fail, so it appears in neither single-clause revert — it is covered by the 8-fail run against `main`.) Full `test/unit_test/data_source/` suite: **3 failed, 199 passed**, and the failure set is byte-identical to clean `main` (**3 failed, 188 passed**) — the 3 are `TestSSRFValidation::*`, which resolve `api.example.com` against real DNS and are unrelated to this change. `ruff check` and `ruff format --check` are clean on both touched files. --- This PR was drafted with AI assistance (Claude). I reviewed the change, independently reproduced both symptoms against `main`, and take responsibility for it.
206 lines
7.7 KiB
Python
206 lines
7.7 KiB
Python
import logging
|
|
import re
|
|
from copy import copy
|
|
from dataclasses import dataclass
|
|
from io import BytesIO
|
|
from typing import IO
|
|
|
|
import bs4
|
|
|
|
from common.data_source.config import (
|
|
HTML_BASED_CONNECTOR_TRANSFORM_LINKS_STRATEGY,
|
|
HtmlBasedConnectorTransformLinksStrategy,
|
|
WEB_CONNECTOR_IGNORED_CLASSES,
|
|
WEB_CONNECTOR_IGNORED_ELEMENTS,
|
|
PARSE_WITH_TRAFILATURA,
|
|
)
|
|
|
|
MINTLIFY_UNWANTED = ["sticky", "hidden"]
|
|
|
|
|
|
@dataclass
|
|
class ParsedHTML:
|
|
title: str | None
|
|
cleaned_text: str
|
|
|
|
|
|
def strip_excessive_newlines_and_spaces(document: str) -> str:
|
|
# collapse repeated spaces into one
|
|
document = re.sub(r" +", " ", document)
|
|
# remove trailing spaces
|
|
document = re.sub(r" +[\n\r]", "\n", document)
|
|
# remove repeated newlines
|
|
document = re.sub(r"[\n\r]+", "\n", document)
|
|
return document.strip()
|
|
|
|
|
|
def strip_newlines(document: str) -> str:
|
|
# HTML might contain newlines which are just whitespaces to a browser
|
|
return re.sub(r"[\n\r]+", " ", document)
|
|
|
|
|
|
def format_element_text(element_text: str, link_href: str | None) -> str:
|
|
element_text_no_newlines = strip_newlines(element_text)
|
|
|
|
if not link_href or HTML_BASED_CONNECTOR_TRANSFORM_LINKS_STRATEGY == HtmlBasedConnectorTransformLinksStrategy.STRIP:
|
|
return element_text_no_newlines
|
|
|
|
return f"[{element_text_no_newlines}]({link_href})"
|
|
|
|
|
|
def parse_html_with_trafilatura(html_content: str) -> str:
|
|
"""Parse HTML content using trafilatura."""
|
|
import trafilatura # type: ignore
|
|
from trafilatura.settings import use_config # type: ignore
|
|
|
|
config = use_config()
|
|
config.set("DEFAULT", "include_links", "True")
|
|
config.set("DEFAULT", "include_tables", "True")
|
|
config.set("DEFAULT", "include_images", "True")
|
|
config.set("DEFAULT", "include_formatting", "True")
|
|
|
|
extracted_text = trafilatura.extract(html_content, config=config)
|
|
return strip_excessive_newlines_and_spaces(extracted_text) if extracted_text else ""
|
|
|
|
|
|
def format_document_soup(document: bs4.BeautifulSoup, table_cell_separator: str = "\t") -> str:
|
|
"""Format html to a flat text document.
|
|
|
|
The following goals:
|
|
- Newlines from within the HTML are removed (as browser would ignore them as well).
|
|
- Repeated newlines/spaces are removed (as browsers would ignore them).
|
|
- Newlines only before and after headlines and paragraphs or when explicit (br or pre tag)
|
|
- Table columns/rows are separated by newline
|
|
- List elements are separated by newline and start with a hyphen
|
|
"""
|
|
text = ""
|
|
list_element_start = False
|
|
verbatim_output = 0
|
|
last_added_newline = False
|
|
|
|
# ``descendants`` yields opening tags only, so a flag set on <table>/<a> would
|
|
# never clear. Precompute scope by registering each <table>/<a> descendant;
|
|
# this avoids O(depth) ancestor walks while keeping lookups O(1).
|
|
table_scope = {id(d) for table in document.find_all("table") for d in table.descendants}
|
|
href_scope = {}
|
|
for anchor in document.find_all("a"): # document order, so a nested <a> wins over its parent
|
|
href_value = anchor.get("href", None)
|
|
# mostly for typing, having multiple hrefs is not valid HTML
|
|
link_href = href_value[0] if isinstance(href_value, list) else href_value
|
|
href_scope.update((id(d), link_href) for d in anchor.descendants)
|
|
|
|
for e in document.descendants:
|
|
verbatim_output -= 1
|
|
in_table = id(e) in table_scope
|
|
link_href = href_scope.get(id(e))
|
|
|
|
if isinstance(e, bs4.element.NavigableString):
|
|
if isinstance(e, (bs4.element.Comment, bs4.element.Doctype)):
|
|
continue
|
|
element_text = e.text
|
|
if in_table:
|
|
# Tables are represented in natural language with rows separated by newlines
|
|
# Can't have newlines then in the table elements
|
|
element_text = element_text.replace("\n", " ").strip()
|
|
|
|
# Some tags are translated to spaces but in the logic underneath this section, we
|
|
# translate them to newlines as a browser should render them such as with br
|
|
# This logic here avoids a space after newline when it shouldn't be there.
|
|
if last_added_newline and element_text.startswith(" "):
|
|
element_text = element_text[1:]
|
|
last_added_newline = False
|
|
|
|
if element_text:
|
|
content_to_add = element_text if verbatim_output > 0 else format_element_text(element_text, link_href)
|
|
|
|
# Don't join separate elements without any spacing
|
|
if (text and not text[-1].isspace()) and (content_to_add and not content_to_add[0].isspace()):
|
|
text += " "
|
|
|
|
text += content_to_add
|
|
|
|
list_element_start = False
|
|
elif isinstance(e, bs4.element.Tag):
|
|
# TR is for rows
|
|
if e.name == "tr" and in_table:
|
|
text += "\n"
|
|
# td for data cell, th for header
|
|
elif e.name in ["td", "th"] and in_table:
|
|
text += table_cell_separator
|
|
elif in_table:
|
|
# don't handle other cases while in table
|
|
pass
|
|
elif e.name in ["p", "div"]:
|
|
if not list_element_start:
|
|
text += "\n"
|
|
elif e.name in ["h1", "h2", "h3", "h4"]:
|
|
text += "\n"
|
|
list_element_start = False
|
|
last_added_newline = True
|
|
elif e.name == "br":
|
|
text += "\n"
|
|
list_element_start = False
|
|
last_added_newline = True
|
|
elif e.name == "li":
|
|
text += "\n- "
|
|
list_element_start = True
|
|
elif e.name == "pre":
|
|
if verbatim_output <= 0:
|
|
verbatim_output = len(list(e.childGenerator()))
|
|
return strip_excessive_newlines_and_spaces(text)
|
|
|
|
|
|
def parse_html_page_basic(text: str | BytesIO | IO[bytes]) -> str:
|
|
soup = bs4.BeautifulSoup(text, "html.parser")
|
|
return format_document_soup(soup)
|
|
|
|
|
|
def web_html_cleanup(
|
|
page_content: str | bs4.BeautifulSoup,
|
|
mintlify_cleanup_enabled: bool = True,
|
|
additional_element_types_to_discard: list[str] | None = None,
|
|
) -> ParsedHTML:
|
|
if isinstance(page_content, str):
|
|
soup = bs4.BeautifulSoup(page_content, "html.parser")
|
|
else:
|
|
soup = page_content
|
|
|
|
title_tag = soup.find("title")
|
|
title = None
|
|
if title_tag and title_tag.text:
|
|
title = title_tag.text
|
|
title_tag.extract()
|
|
|
|
# Heuristics based cleaning of elements based on css classes
|
|
unwanted_classes = copy(WEB_CONNECTOR_IGNORED_CLASSES)
|
|
if mintlify_cleanup_enabled:
|
|
unwanted_classes.extend(MINTLIFY_UNWANTED)
|
|
for undesired_element in unwanted_classes:
|
|
[tag.extract() for tag in soup.find_all(class_=lambda x: x and undesired_element in x.split())]
|
|
|
|
for undesired_tag in WEB_CONNECTOR_IGNORED_ELEMENTS:
|
|
[tag.extract() for tag in soup.find_all(undesired_tag)]
|
|
|
|
if additional_element_types_to_discard:
|
|
for undesired_tag in additional_element_types_to_discard:
|
|
[tag.extract() for tag in soup.find_all(undesired_tag)]
|
|
|
|
soup_string = str(soup)
|
|
page_text = ""
|
|
|
|
if PARSE_WITH_TRAFILATURA:
|
|
try:
|
|
page_text = parse_html_with_trafilatura(soup_string)
|
|
if not page_text:
|
|
raise ValueError("Empty content returned by trafilatura.")
|
|
except Exception as e:
|
|
logging.info(f"Trafilatura parsing failed: {e}. Falling back on bs4.")
|
|
page_text = format_document_soup(soup)
|
|
else:
|
|
page_text = format_document_soup(soup)
|
|
|
|
# 200B is ZeroWidthSpace which we don't care for
|
|
cleaned_text = page_text.replace("\u200b", "")
|
|
|
|
return ParsedHTML(title=title, cleaned_text=cleaned_text)
|