Keep the listing container off forum threads
Thread pages line up their posts as sibling <article> elements too, so in recall mode the container took every post along with its user card and quotes (katzen-forum.net, XenForo). The container is now used only when every entry is a teaser: its first heading links to a page of its own, which no other entry links to. Poster names come back along a thread or carry no link. On top of that it is never used on pages already detected as thread forums (DiscussionForumPosting).
This commit is contained in:
1 parent
03e823a030
commit
77465a50e7
3 files changed
+91
-12
No files matched your search
@@ -2953,6 +2953,62 @@ def test_preloaded_full_articles_are_not_a_listing(settings):
|
||||
assert "Geldstrafe für die AfD" not in result
|
||||
|
||||
|
||||
def test_listing_page_with_links_keeps_every_entry_heading():
|
||||
"with links kept the heading targets live in another attribute, the listing must still be found"
|
||||
entries = _listing_entries()
|
||||
doc = _listing_page(entries)
|
||||
result = extract(doc, output_format="markdown", include_links=True, config=use_config()) or ""
|
||||
for i, (title, text) in enumerate(entries):
|
||||
assert f"## [{title}](/{i})" in result
|
||||
assert text in result
|
||||
|
||||
|
||||
def _forum_thread(posters, linked=True, forum_markup=False):
|
||||
"Thread markup: sibling <article> posts, each opening with a user card that names its poster in a heading."
|
||||
name_markup = '<a href="/members/{0}/">{0}</a>' if linked else "<span>{0}</span>"
|
||||
posts = "".join(
|
||||
f'<article><div class="poster"><h4>{name_markup.format(name)}</h4>'
|
||||
"<dl><dt>Joined</dt><dd>March 2015</dd><dt>Messages</dt><dd>1,447</dd></dl></div>"
|
||||
f"<div><p>Reply {i} from {name} carries enough prose to count as a post, running over a couple "
|
||||
"of clauses so the thread reads as a real discussion between people.</p></div></article>"
|
||||
for i, name in enumerate(posters)
|
||||
)
|
||||
json_ld = (
|
||||
'<script type="application/ld+json">{"@context": "https://schema.org", '
|
||||
'"@type": "DiscussionForumPosting", "headline": "A thread"}</script>'
|
||||
if forum_markup
|
||||
else ""
|
||||
)
|
||||
return (
|
||||
f"<html><head>{json_ld}</head><body><div id='wrapper'><main>"
|
||||
f"<div class='thread'>{posts}</div></main></div></body></html>"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"posters,linked,forum_markup",
|
||||
[
|
||||
# the same poster comes back along the thread
|
||||
(["alice", "bob", "alice", "carol", "bob", "alice"], True, False),
|
||||
# poster names without a link
|
||||
(["alice", "bob", "carol", "dave", "erin", "frank"], False, False),
|
||||
# every poster different, but the page declares itself a forum thread
|
||||
(["alice", "bob", "carol", "dave", "erin", "frank"], True, True),
|
||||
],
|
||||
)
|
||||
@pytest.mark.parametrize("settings", [{"favor_recall": True}, {"favor_recall": True, "fast": True}])
|
||||
def test_forum_thread_is_not_a_listing(posters, linked, forum_markup, settings):
|
||||
"forum posts line up like listing entries, but taking their container brings every user card along"
|
||||
doc = _forum_thread(posters, linked, forum_markup)
|
||||
with _without_listing_expression():
|
||||
expected = extract(doc, output_format="markdown", config=use_config(), **settings)
|
||||
result = extract(doc, output_format="markdown", config=use_config(), **settings) or ""
|
||||
assert result == expected
|
||||
assert result.count("Joined") < len(posters)
|
||||
for i in range(len(posters)):
|
||||
assert f"Reply {i} from" in result
|
||||
|
||||
|
||||
def test_short_document_keeps_structure():
|
||||
"regression #896: the baseline rescue must not flatten a valid short extraction it cannot improve on."
|
||||
doc = (
|
||||
|
||||
+6
-6
@@ -149,21 +149,21 @@ def _prepare_tree(tree: HtmlElement, options: Extractor, url: str | None) -> tup
|
||||
|
||||
|
||||
def _extract_and_compare(
|
||||
cleaned_tree: HtmlElement, cleaned_tree_backup: HtmlElement, tree: HtmlElement, options: Extractor
|
||||
cleaned_tree: HtmlElement, cleaned_tree_backup: HtmlElement, tree: HtmlElement, options: Extractor, is_forum: bool
|
||||
) -> tuple[_Element, str]:
|
||||
"Cascade stages 1-2: main extractor, then the external comparison unless in fast mode."
|
||||
postbody, temp_text = extract_content(cleaned_tree, options)
|
||||
postbody, temp_text = extract_content(cleaned_tree, options, is_forum)
|
||||
if not options.fast:
|
||||
postbody, temp_text = compare_extraction(cleaned_tree_backup, copy(tree), postbody, temp_text, options)
|
||||
return postbody, temp_text
|
||||
|
||||
|
||||
def _recall_retry(esc_tree: HtmlElement, r_options: Extractor, url: str | None) -> tuple[_Element, str]:
|
||||
def _recall_retry(esc_tree: HtmlElement, r_options: Extractor, url: str | None, is_forum: bool) -> tuple[_Element, str]:
|
||||
"""Stage-4 retry: re-run cascade stages 1-2 in recall mode on the escalation input
|
||||
(arrives comment-pruned, or intact on a thread-forum where posts are content).
|
||||
Deliberately no comment capture, no baseline (it already ran on the full page; on a
|
||||
comment-pruned tree it only yields an indistinguishable boilerplate dump), no escalation."""
|
||||
return _extract_and_compare(*_prepare_tree(esc_tree, r_options, url), esc_tree, r_options)
|
||||
return _extract_and_compare(*_prepare_tree(esc_tree, r_options, url), esc_tree, r_options, is_forum)
|
||||
|
||||
|
||||
def trafilatura_sequence(
|
||||
@@ -209,7 +209,7 @@ def trafilatura_sequence(
|
||||
# and match the xpath's self::list (dl lists, disqus ids, and all lists if comments are on)
|
||||
cleaned_tree = prune_unwanted_nodes(cleaned_tree, REMOVE_COMMENTS_XPATH)
|
||||
|
||||
postbody, temp_text = _extract_and_compare(cleaned_tree, cleaned_tree_backup, tree, options)
|
||||
postbody, temp_text = _extract_and_compare(cleaned_tree, cleaned_tree_backup, tree, options, is_forum)
|
||||
|
||||
# 3. rescue: baseline on the original tree, accepted only if it adds text (#896)
|
||||
if len(temp_text) < options.min_extracted_size and options.focus != "precision":
|
||||
@@ -240,7 +240,7 @@ def trafilatura_sequence(
|
||||
)
|
||||
r_text = ""
|
||||
try:
|
||||
r_body, r_text = _recall_retry(esc_tree, r_options, url)
|
||||
r_body, r_text = _recall_retry(esc_tree, r_options, url, is_forum)
|
||||
except Exception as err: # pragma: no cover
|
||||
LOGGER.warning("recall retry failed: %s %s", err, url)
|
||||
# justext reaches div-buried content the rule retry misses (gated: ungated regressed
|
||||
|
||||
@@ -662,12 +662,31 @@ def _holds_most_of_the_text(tree: HtmlElement, expr: XPath) -> bool:
|
||||
return len(trim(" ".join(match.itertext()))) / page_length >= LISTING_TEXT_SHARE
|
||||
|
||||
|
||||
def _entries_link_out(container: _Element) -> bool:
|
||||
"""Whether every entry of the container is a teaser: its first heading links to a page of its
|
||||
own, which no other entry links to. Forum posts share the shape but open with the poster's
|
||||
name, which comes back along the thread or carries no link at all."""
|
||||
targets: set[str] = set()
|
||||
for entry in container.iterchildren("article"):
|
||||
heading = entry.find(".//head")
|
||||
link = heading.find(".//ref") if heading is not None else None
|
||||
if link is None:
|
||||
return False
|
||||
# convert_tags keeps the href when links are off and moves it to target when they are on
|
||||
target = link.get("href") or link.get("target")
|
||||
if not target or target in targets:
|
||||
return False
|
||||
targets.add(target)
|
||||
return True
|
||||
|
||||
|
||||
def _is_listing_page(tree: HtmlElement, options: Extractor) -> bool:
|
||||
"""Whether the sibling-article container is the body of a listing page. The container only
|
||||
widens what BODY_XPATH would take on its own: that element has to be one of its entries and a
|
||||
short teaser. A body outside the container means a teaser strip beside the article, and an
|
||||
entry that already clears the recall "sure thing" length is a full article, as on pages that
|
||||
preload the next stories below the current one."""
|
||||
preload the next stories below the current one. Every entry also has to be a teaser that
|
||||
links out from its heading."""
|
||||
listing_expr = LISTING_BODY_XPATH[0]
|
||||
container = next((s for s in listing_expr(tree) if s is not None), None)
|
||||
if container is None:
|
||||
@@ -677,10 +696,12 @@ def _is_listing_page(tree: HtmlElement, options: Extractor) -> bool:
|
||||
return False
|
||||
if len(trim(" ".join(entry.itertext()))) > options.min_extracted_size * 10:
|
||||
return False
|
||||
if not _entries_link_out(container):
|
||||
return False
|
||||
return _holds_most_of_the_text(tree, listing_expr)
|
||||
|
||||
|
||||
def _extract(tree: HtmlElement, options: Extractor) -> tuple[_Element, str, set[str]]:
|
||||
def _extract(tree: HtmlElement, options: Extractor, is_forum: bool = False) -> tuple[_Element, str, set[str]]:
|
||||
# init
|
||||
potential_tags = set(TAG_CATALOG)
|
||||
if options.tables is True:
|
||||
@@ -691,7 +712,8 @@ def _extract(tree: HtmlElement, options: Extractor) -> tuple[_Element, str, set[
|
||||
potential_tags.add("ref")
|
||||
result_body = Element("body")
|
||||
expressions = BODY_XPATH
|
||||
if options.focus == "recall" and _is_listing_page(tree, options):
|
||||
# the posts of a thread forum line up the same way, each with its user card and quotes
|
||||
if options.focus == "recall" and not is_forum and _is_listing_page(tree, options):
|
||||
expressions = LISTING_BODY_XPATH + BODY_XPATH
|
||||
# iterate
|
||||
for expr in expressions:
|
||||
@@ -760,16 +782,17 @@ def _extract(tree: HtmlElement, options: Extractor) -> tuple[_Element, str, set[
|
||||
return result_body, temp_text, potential_tags
|
||||
|
||||
|
||||
def extract_content(cleaned_tree: HtmlElement, options: Extractor) -> tuple[_Element, str]:
|
||||
def extract_content(cleaned_tree: HtmlElement, options: Extractor, is_forum: bool = False) -> tuple[_Element, str]:
|
||||
"""Find the main content of a page using a set of XPath expressions,
|
||||
then extract relevant elements, strip them of unwanted subparts and
|
||||
convert them"""
|
||||
convert them. is_forum marks a thread forum, whose posts are never
|
||||
read as the entries of a listing page."""
|
||||
# backup
|
||||
backup_tree = deepcopy(cleaned_tree)
|
||||
# source order before the main pass moves elements, the backup keeps it
|
||||
order = list(cleaned_tree.iter())
|
||||
|
||||
result_body, temp_text, potential_tags = _extract(cleaned_tree, options)
|
||||
result_body, temp_text, potential_tags = _extract(cleaned_tree, options, is_forum)
|
||||
|
||||
# try parsing wild <p> elements if nothing found or text too short
|
||||
# todo: test precision and recall settings here
|
||||
|
||||
Reference in new issue
Block a user