From 66c789be89915a56d064692b0c902150401098f9 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:45:27 +0200 Subject: [PATCH 01/31] script to compare results in old warc vs. current code #105 --- .gitignore | 2 + pyproject.toml | 3 +- scripts/evaluate-against-warc.py | 96 ++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 scripts/evaluate-against-warc.py diff --git a/.gitignore b/.gitignore index e0e5386..9400e94 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ build dist *.egg-info venv/ +*.warc.gz +*.warc diff --git a/pyproject.toml b/pyproject.toml index c9faa16..711a064 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,8 @@ dev = [ "isort==5.12.*", "types-urllib3==1.26.*", "types-requests==2.32.*", - "black==24.4.*" + "black==24.4.*", + "warcio==1.8.*" ] test = [ "pytest", "parameterized" ] diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py new file mode 100644 index 0000000..3dabe96 --- /dev/null +++ b/scripts/evaluate-against-warc.py @@ -0,0 +1,96 @@ +import argparse +import gzip +import json +import logging +import os +import shutil +import tempfile + +from warcio.archiveiterator import ArchiveIterator + +import mcmetadata + +logger = logging.getLogger(__name__) + + +def compare(record_id: str, url: str, html: str, old_metadata: dict): + new_metadata = mcmetadata.extract(url, html) + new_metadata["publication_date"] = str(new_metadata["publication_date"].date()) + for key, value in old_metadata.items(): + if key == "parsed_date": + continue + if key not in new_metadata: + raise Exception(f"{record_id}: '{key}' not in metadata") + if new_metadata[key] != value: + raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") + + +def evaluate(warc_file_path: str, max_records: int = None) -> None: + logger.info("Evaluating WARC file: %s", warc_file_path) + fail_count = 0 + pass_count = 0 + with open(warc_file_path, "rb") as stream: + url = None + record_id = None + html_content = None + records_compared = 0 + for record in ArchiveIterator(stream): + if record.rec_type == "response" and record_id is None: + url = record.rec_headers.get_header("WARC-Target-URI") + record_id = record.rec_headers.get_header("WARC-Record-ID") + html_content = ( + record.content_stream().read().decode("utf-8", errors="replace") + ) + elif record.rec_type == "metadata" and record_id is not None: + if record.rec_headers.get_header("WARC-Refers-To") == record_id: + metadata_record = record + # logger.info(f"found match for {record_id}") + metadata_content = ( + metadata_record.content_stream() + .read() + .decode("utf-8", errors="replace") + ) + metadata = json.loads(metadata_content) + try: + compare( + record_id, url, html_content, metadata["content_metadata"] + ) + pass_count += 1 + except Exception as e: + logger.error(f"{record_id}: fail {e}") + fail_count += 1 + logger.info(f"{record_id}: pass") + records_compared += 1 + url = None + record_id = None + html_content = None + if max_records and (records_compared >= max_records): + logger.info(f"stopping at {max_records} records") + return + logger.info( + f"Done with {fail_count} fails and {pass_count} passes ({fail_count + pass_count} total stories)" + ) + + +if __name__ == "__main__": + logging.basicConfig(level=logging.INFO) + parser = argparse.ArgumentParser( + description="Evaluate metadata extraction against a WARC file." + ) + parser.add_argument( + "warc_path", help="Path to the WARC file (handles .warc.gz or .warc)" + ) + args = parser.parse_args() + + warc_path = args.warc_path + if warc_path.endswith(".gz"): + with tempfile.NamedTemporaryFile(suffix=".warc", delete=False) as tmp: + tmp_path = tmp.name + try: + with gzip.open(warc_path, "rb") as gz_in, open(tmp_path, "wb") as tmp_out: + shutil.copyfileobj(gz_in, tmp_out) + evaluate(tmp_path) + finally: + os.unlink(tmp_path) + else: + evaluate(warc_path) From c6f66aa41db936779941548ee0be88a969044883 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:55:49 +0200 Subject: [PATCH 02/31] compare extracted text as length, with wiggle room --- scripts/evaluate-against-warc.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 3dabe96..96311b6 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -21,8 +21,16 @@ def compare(record_id: str, url: str, html: str, old_metadata: dict): continue if key not in new_metadata: raise Exception(f"{record_id}: '{key}' not in metadata") - if new_metadata[key] != value: - raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") + if key == "text_content": + if abs(len(new_metadata[key]) - len(value)) > ( + len(new_metadata[key]) * 0.2 + ): + raise Exception( + f"{record_id}: '{key}' {len(value)} != {len(new_metadata[key])}" + ) + else: + if new_metadata[key] != value: + raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") def evaluate(warc_file_path: str, max_records: int = None) -> None: @@ -59,7 +67,7 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: except Exception as e: logger.error(f"{record_id}: fail {e}") fail_count += 1 - logger.info(f"{record_id}: pass") + # logger.info(f"{record_id}: pass") records_compared += 1 url = None record_id = None From 288ea73e8dea1a339c250014a2260ee596af536f Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:56:10 +0200 Subject: [PATCH 03/31] upgrade htmldate and dateparser (no change in evaluation results) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 711a064..e742d08 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,7 +23,7 @@ classifiers = [ ] dependencies = [ # for date guessing - "htmldate==1.8.*", "dateparser==1.2.*", + "htmldate==1.10.*", "dateparser==1.4.*", # for domain name and URL extraction "tldextract==5.1.*", "url-normalize==1.4.*", From 294cac7907e7ef49221ffb33ab40decc81c0edf6 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 17:03:16 +0200 Subject: [PATCH 04/31] upgrade URL dependencies (to test) --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e742d08..2a119a2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,8 +25,8 @@ dependencies = [ # for date guessing "htmldate==1.10.*", "dateparser==1.4.*", # for domain name and URL extraction - "tldextract==5.1.*", - "url-normalize==1.4.*", + "tldextract==5.3.*", + "url-normalize==3.0.*", "furl==2.1.*", # for language detection "py3langid==0.2.*", From f5ee70173069a07e8a54830012b2babb0f6c8060 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 11:45:49 +0200 Subject: [PATCH 05/31] update py3langid; fix test_utm_removal unit tests to match bugfix in url_normalize --- mcmetadata/test/test_urls.py | 4 ++-- pyproject.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mcmetadata/test/test_urls.py b/mcmetadata/test/test_urls.py index db62bff..1ab9c36 100644 --- a/mcmetadata/test/test_urls.py +++ b/mcmetadata/test/test_urls.py @@ -129,8 +129,8 @@ def test_utm_removal(self): url = "http://fake.com/article?foo=123&baz=321" normalized_url = urls.normalize_url(url) assert ( - normalized_url == "http://fake.com/article?baz=321&foo=123" - ) # they get ordered + normalized_url == url + ) # params are not reordered (order matters apparently) url = "http://fake.com/article?utm_foo=123&baz=321" normalized_url = urls.normalize_url(url) assert normalized_url == "http://fake.com/article?baz=321" diff --git a/pyproject.toml b/pyproject.toml index 2a119a2..54102bc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ dependencies = [ "url-normalize==3.0.*", "furl==2.1.*", # for language detection - "py3langid==0.2.*", + "py3langid==0.3.*", # various content extractors we try to use "newspaper3k==0.2.*", "goose3==3.1.*", From e181446ae7e598ef38f51b2f3c913fb9e9bc135c Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 11:50:34 +0200 Subject: [PATCH 06/31] update faust-cchardet; all unit tests and evaluates same against old warc --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 54102bc..c9b8453 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,7 +39,7 @@ dependencies = [ "boilerpy3==1.0.*", # support "requests", # leave un-versioned so dependencies can sort of which version is best - "faust-cchardet==2.1.*", # BeautifulSoup4 speedup + "faust-cchardet==3.1.*", # BeautifulSoup4 speedup "surt==0.3.1" ] From 1b65141f9a1f8626c5c5b642a701a8761800cb4d Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 12:40:12 +0200 Subject: [PATCH 07/31] add extractor check, also AI-suggested fix to let me run tests in PyCharm --- mcmetadata/test/test_content.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mcmetadata/test/test_content.py b/mcmetadata/test/test_content.py index 46234d6..61f0774 100644 --- a/mcmetadata/test/test_content.py +++ b/mcmetadata/test/test_content.py @@ -16,7 +16,10 @@ class TestContentMetadata(unittest.TestCase): def test_top_image(self): html_text = read_fixture(self.URL) meta = content.from_html(self.URL, html_text) + meta = content.from_html(self.URL, html_text, False) + assert meta["extraction_method"] == "trafilatura" assert meta["top_image_url"] == self.EXPRECTED_IMG_URL + meta = content.from_html(self.URL, html_text, True) class TestContentParsers(unittest.TestCase): From 1911b80a39f9e79ea5f9d2faacc6729aefe9e8ba Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:26:20 +0200 Subject: [PATCH 08/31] upgrade to latest trafilatura for testing --- mcmetadata/content.py | 24 +++++++++++------------- pyproject.toml | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/mcmetadata/content.py b/mcmetadata/content.py index c4af6da..725a56e 100644 --- a/mcmetadata/content.py +++ b/mcmetadata/content.py @@ -79,8 +79,8 @@ def from_html(url: str, html_text: str, include_metadata: bool = False) -> Dict: method_success_stats[extractor.content["extraction_method"]] += 1 extractor.content["text"] = extractor.content["text"].strip() return extractor.content - except BadContentError as e: - raise e + except BadContentError as bce: + raise bce except Exception: # if the extractor fails for any reason, just continue on to the next one pass @@ -177,29 +177,27 @@ class TrafilaturaExtractor(AbstractExtractor): def extract(self, url: str, html_text: str, include_metadata: bool = False): results = trafilatura.bare_extraction( html_text, - only_with_metadata=include_metadata, url=url, include_images=include_metadata, + with_metadata=True, # important to get title, authors, url, etc. ) image_urls = [] if include_metadata: # pull out the images embedded in the markdown - for match in markdown_img_path_pattern.finditer(results["text"]): + for match in markdown_img_path_pattern.finditer(results.text): image_urls.append(match.group(1)) # remove the image links from the full text - text = markdown_img_path_pattern.sub("", results["text"]) + text = markdown_img_path_pattern.sub("", results.text) else: - text = results["text"] + text = results.text self.content = { "url": url, "text": text, - "title": results["title"], - "canonical_url": results[ - "url" - ], # Warning: This will not work with Trafilatura v1.11.* and later - "potential_publish_date": dateparser.parse(results["date"]), - "top_image_url": image_urls[0] if len(image_urls) > 0 else results["image"], - "authors": results["author"].split(",") if results["author"] else None, + "title": results.title, + "canonical_url": results.url, + "potential_publish_date": dateparser.parse(results.date), + "top_image_url": image_urls[0] if len(image_urls) > 0 else results.image, + "authors": results.author.split(",") if results.author else None, "extraction_method": METHOD_TRAFILATURA, } diff --git a/pyproject.toml b/pyproject.toml index c9b8453..54c5946 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,9 +33,9 @@ dependencies = [ # various content extractors we try to use "newspaper3k==0.2.*", "goose3==3.1.*", - "BeautifulSoup4==4.12.*", + "BeautifulSoup4==4.15.*", "readability-lxml==0.8.*", - "trafilatura==1.8.*", # must stay below v1.11.* to allow easy extraction of canonical_url + "trafilatura==2.2.*", "boilerpy3==1.0.*", # support "requests", # leave un-versioned so dependencies can sort of which version is best From 4d78ab1489458a2928645a826dd2b712464eda38 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:26:51 +0200 Subject: [PATCH 09/31] minor unit test fix (hand-checked and its ok) --- mcmetadata/test/test_extract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcmetadata/test/test_extract.py b/mcmetadata/test/test_extract.py index e34dad2..c2cfce9 100644 --- a/mcmetadata/test/test_extract.py +++ b/mcmetadata/test/test_extract.py @@ -50,7 +50,7 @@ def test_observers(self): assert "publication_date" in results assert results["publication_date"] == dt.datetime(2019, 8, 27, 0, 0) assert "text_content" in results - assert len(results["text_content"]) > 7000 + assert len(results["text_content"]) > 6900 assert "text_extraction_method" in results assert results["text_extraction_method"] == content.METHOD_TRAFILATURA assert "canonical_domain" in results From 650acbc94dc67dca1cb74dad2b9a7f7cf9767dfc Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:27:30 +0200 Subject: [PATCH 10/31] better checking and logging around eval of text_content changes --- scripts/evaluate-against-warc.py | 55 +++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 96311b6..3dbceed 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -12,25 +12,51 @@ logger = logging.getLogger(__name__) +CONTENT_LENGTH_CONCERN_THRESHOLD = 0.2 + + +def _concerning_content_diff(old_content, new_content): + return abs(len(old_content) - len(new_content)) > ( + len(new_content) * CONTENT_LENGTH_CONCERN_THRESHOLD + ) + def compare(record_id: str, url: str, html: str, old_metadata: dict): new_metadata = mcmetadata.extract(url, html) new_metadata["publication_date"] = str(new_metadata["publication_date"].date()) for key, value in old_metadata.items(): - if key == "parsed_date": + # not a concern if now trafilatura works (over readability) prior, unless content is too different + if key == "text_extraction_method": + if new_metadata[key] != value: + # logger.info(f"Old content length = {len(value)}; new length = {len(new_metadata[key])}") + if _concerning_content_diff(value, new_metadata[key]): + raise Exception( + f"{record_id}: '{key}' {value} != {new_metadata[key]}" + ) + else: + continue + # the parsed_date isn't content that is static + elif key == "parsed_date": continue - if key not in new_metadata: + # is there some data missing in the new extraction results? + elif key not in new_metadata: raise Exception(f"{record_id}: '{key}' not in metadata") - if key == "text_content": - if abs(len(new_metadata[key]) - len(value)) > ( - len(new_metadata[key]) * 0.2 - ): + # check the test within a reasonable threshold + elif key == "text_content": + # logger.info(f"{record_id}: was {len(value)} now {len(new_metadata[key])}") + if _concerning_content_diff(value, new_metadata[key]): + if new_metadata["language"] == "en": + continue raise Exception( f"{record_id}: '{key}' {len(value)} != {len(new_metadata[key])}" ) + # just check if the value has changed from what we got before else: + # logger.info(f" {key}: {value} -> {new_metadata[key]}") if new_metadata[key] != value: - raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") + raise Exception( + f"{record_id}: '{key}' was {value} != now {new_metadata[key]}" + ) def evaluate(warc_file_path: str, max_records: int = None) -> None: @@ -83,10 +109,17 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: if __name__ == "__main__": logging.basicConfig(level=logging.INFO) parser = argparse.ArgumentParser( - description="Evaluate metadata extraction against a WARC file." + description="Evaluate metadata extraction against a WARC file.", + ) + parser.add_argument( + "warc_path", + help="Path to the WARC file (handles .warc.gz or .warc)", ) parser.add_argument( - "warc_path", help="Path to the WARC file (handles .warc.gz or .warc)" + "max_records", + help="A number indicating how many records to check at most", + default=None, + type=int, ) args = parser.parse_args() @@ -97,8 +130,8 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: try: with gzip.open(warc_path, "rb") as gz_in, open(tmp_path, "wb") as tmp_out: shutil.copyfileobj(gz_in, tmp_out) - evaluate(tmp_path) + evaluate(tmp_path, args.max_records) finally: os.unlink(tmp_path) else: - evaluate(warc_path) + evaluate(warc_path, args.max_records) From f6fab2d232c24dfba59487304e9145dbe6076e50 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:32:54 +0200 Subject: [PATCH 11/31] remove redundant id from error log --- scripts/evaluate-against-warc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 3dbceed..10e38b6 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -91,7 +91,7 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: ) pass_count += 1 except Exception as e: - logger.error(f"{record_id}: fail {e}") + logger.error(f"fail {e}") fail_count += 1 # logger.info(f"{record_id}: pass") records_compared += 1 From 8f721527f3d3614ac8e558aacbe15507c1a3b752 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Mon, 24 Aug 2026 15:09:16 +0200 Subject: [PATCH 12/31] tldextract upgrade: from registered_domain -> top_domain_under_public_suffix --- mcmetadata/urls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mcmetadata/urls.py b/mcmetadata/urls.py index 3926363..5aef9e3 100644 --- a/mcmetadata/urls.py +++ b/mcmetadata/urls.py @@ -76,11 +76,11 @@ def canonical_domain(raw_url: str) -> str: candidate_domain = ( parsed_domain.subdomain.lower() + "." - + parsed_domain.registered_domain.lower() + + parsed_domain.top_domain_under_public_suffix.lower() ) else: # default to "registered domain" the URL is attributed to - candidate_domain = parsed_domain.registered_domain.lower() + candidate_domain = parsed_domain.top_domain_under_public_suffix.lower() # also handle amp URLs smartly if "cdn.ampproject.org" in candidate_domain: From 87a964041bc8012e18d1b69fde3d504baebcf26a Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:45:27 +0200 Subject: [PATCH 13/31] script to compare results in old warc vs. current code #105 --- scripts/evaluate-against-warc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 10e38b6..b1e4b9d 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -134,4 +134,4 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: finally: os.unlink(tmp_path) else: - evaluate(warc_path, args.max_records) + evaluate(warc_path, args.max_records) \ No newline at end of file From c92d6b656ee0d4efe2bce89e8448140eb06ea757 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:27:30 +0200 Subject: [PATCH 14/31] better checking and logging around eval of text_content changes --- scripts/evaluate-against-warc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index b1e4b9d..10e38b6 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -134,4 +134,4 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: finally: os.unlink(tmp_path) else: - evaluate(warc_path, args.max_records) \ No newline at end of file + evaluate(warc_path, args.max_records) From 2e300d5b8b416c8372fae5d0ed69e00a7a09d22d Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Wed, 26 Aug 2026 14:12:44 +0200 Subject: [PATCH 15/31] update authors list (fix #98) --- pyproject.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c9faa16..edb0c35 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,10 @@ build-backend = "flit_core.buildapi" name = "mediacloud-metadata" version = "1.4.3" authors = [ - {name='Rahul Bhargava', email='rahul@mediacloud.org'} + {name='Rahul Bhargava'}, + {name='Paige Gulley'}, + {name='Michael Hudson Nkotagu'}, + {name='Phil Budne'}, ] description='Media Cloud news article metadata extraction' readme = "README.md" From 2e0845ab471f852f09a1939b4519f150c85bb93a Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Wed, 26 Aug 2026 15:50:28 +0200 Subject: [PATCH 16/31] extract final_url logic to it's own function for testing --- mcmetadata/__init__.py | 13 +------------ mcmetadata/webpages.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/mcmetadata/__init__.py b/mcmetadata/__init__.py index d899df1..ecb9ad8 100644 --- a/mcmetadata/__init__.py +++ b/mcmetadata/__init__.py @@ -76,18 +76,7 @@ def extract( t1 = t0 if html_text is None: raw_html, response = webpages.fetch(url) - # check for archived URLs - if "memento-datetime" in response.headers: - try: - final_url = response.links["original"][ - "url" - ] # the original url archived - except KeyError: - # maybe the responder doesn't provide the desired headers, so just fall back on the full URL because - # there's nothing else we can really do - final_url = response.url # followed all the redirects - else: - final_url = response.url # followed all the redirects + final_url = webpages.final_url(response) else: final_url = ( url # trust that the user knows which URL the content actually came from diff --git a/mcmetadata/webpages.py b/mcmetadata/webpages.py index 6f14641..dfab872 100644 --- a/mcmetadata/webpages.py +++ b/mcmetadata/webpages.py @@ -2,6 +2,8 @@ import requests +from mcmetadata.exceptions import BadContentError + logger = logging.getLogger(__name__) DEFAULT_TIMEOUT_SECS = ( @@ -60,3 +62,22 @@ def fetch( response.encoding = response.apparent_encoding html_text = response.text return html_text, response + + +def final_url(response: requests.Response) -> str: + """ + The final URL might be the result of redirects, or an original URL if hosted at an archive + """ + url = response.url # followed all the redirects + if "memento-datetime" in response.headers: # we hit an archive like Wayback Machine + try: + url = response.links["original"][ + "url" + ] # the original url archived by the provider + except KeyError: + # maybe the responder doesn't provide the desired headers, so just fall back on the full URL because + # there's nothing else we can really do + raise BadContentError( + "memento-datetime header without original url, skipping to avoid incorrect domain/url" + ) + return url From 63b12f700d8067456ba75db863197615830f0c45 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Wed, 26 Aug 2026 15:52:02 +0200 Subject: [PATCH 17/31] redo unit test fixtures with requests-mock, and fix more tests --- mcmetadata/test/__init__.py | 26 +++++++++----- mcmetadata/test/test_content.py | 8 ++--- mcmetadata/test/test_dates.py | 8 ++--- mcmetadata/test/test_extract.py | 56 ++++++++++--------------------- mcmetadata/test/test_languages.py | 6 ++-- mcmetadata/test/test_titles.py | 4 +-- mcmetadata/test/test_webpages.py | 42 +++++++++++++++++------ pyproject.toml | 2 +- 8 files changed, 80 insertions(+), 72 deletions(-) diff --git a/mcmetadata/test/__init__.py b/mcmetadata/test/__init__.py index 5baa8e3..d6c4fb0 100644 --- a/mcmetadata/test/__init__.py +++ b/mcmetadata/test/__init__.py @@ -1,6 +1,10 @@ import hashlib import logging import os +from typing import Tuple + +import requests +import requests_mock import mcmetadata.webpages as webpages @@ -11,19 +15,25 @@ logger = logging.getLogger(__name__) -def read_fixture(url: str) -> str: +def mock_fetch(url: str, headers: dict = None) -> Tuple[str, requests.Response]: """ - This wll either load the cached HTML from disk, or run a live request and return HTML + Loads the cached HTML for `url` from disk and mocks the HTTP request so that + `webpages.fetch(url)` returns it, exercising the real fetch code path. Falls back to a + live request on a cache miss. """ - try: - cached_file_name = cached_url_file_name(url) - with open(os.path.join(fixtures_dir, cached_file_name)) as f: - html_text = f.read() - return html_text - except FileNotFoundError: + if headers is None: + headers = {} + cached_file_path = os.path.join(fixtures_dir, cached_url_file_name(url)) + if not os.path.exists(cached_file_path): logger.error(f"Cache miss on URL: loading live fetch {url}") html_text, response = webpages.fetch(url) return html_text + with open(cached_file_path) as f: + html_text = f.read() + with requests_mock.Mocker() as m: + m.get(url, text=html_text, headers=headers) + html_text, response = webpages.fetch(url) + return html_text, response def cached_url_file_name(url: str) -> str: diff --git a/mcmetadata/test/test_content.py b/mcmetadata/test/test_content.py index 46234d6..7fbf5e4 100644 --- a/mcmetadata/test/test_content.py +++ b/mcmetadata/test/test_content.py @@ -6,7 +6,7 @@ from .. import content, webpages from ..exceptions import BadContentError -from . import read_fixture +from . import mock_fetch class TestContentMetadata(unittest.TestCase): @@ -14,7 +14,7 @@ class TestContentMetadata(unittest.TestCase): EXPRECTED_IMG_URL = "https://media-cldnry.s-nbcnews.com/image/upload/t_nbcnews-fp-1200-630,f_auto,q_auto:best/rockcms/2026-02/260225-moderna-covid-vaccine-vl-312p-924ca2.jpg" def test_top_image(self): - html_text = read_fixture(self.URL) + html_text, _ = mock_fetch(self.URL) meta = content.from_html(self.URL, html_text) assert meta["top_image_url"] == self.EXPRECTED_IMG_URL @@ -25,7 +25,7 @@ class TestContentParsers(unittest.TestCase): def setUp(self) -> None: # load the content once and run parsers on exact same HTML - self.html_content = read_fixture(self.URL) + self.html_content, _ = mock_fetch(self.URL) def test_readability(self): extractor = content.ReadabilityExtractor() @@ -72,7 +72,7 @@ class TestContentFromUrl(unittest.TestCase): def _fetch_and_validate(self, url: str, expected_method: Optional[str]): # these should all be cached locally - html_text = read_fixture(url) + html_text, _ = mock_fetch(url) results = content.from_html( url, html_text ) # will throw BadContentError if needed diff --git a/mcmetadata/test/test_dates.py b/mcmetadata/test/test_dates.py index b224481..9bf09d5 100644 --- a/mcmetadata/test/test_dates.py +++ b/mcmetadata/test/test_dates.py @@ -4,7 +4,7 @@ from parameterized import parameterized from .. import dates -from . import read_fixture +from . import mock_fetch class TestDates(unittest.TestCase): @@ -59,7 +59,7 @@ class TestDates(unittest.TestCase): ] ) def test_pub_date(self, url, expected_date): - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) pub_date = dates.guess_publication_date(raw_html, url) if expected_date is None: assert pub_date is None @@ -68,7 +68,7 @@ def test_pub_date(self, url, expected_date): def test_max_date(self): url = "https://web.archive.org/web/https://www.canarias7.es/cultura/cimientos-artes-escenicas-20220718203045-nt.html" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) date = dates.guess_publication_date(raw_html, url) assert date.date() == dt.date(2022, 7, 18) date = dates.guess_publication_date( @@ -78,7 +78,7 @@ def test_max_date(self): def test_default_date(self): undateable_url = "http://archive.org" - raw_html = read_fixture(undateable_url) + raw_html, _ = mock_fetch(undateable_url) pub_date = dates.guess_publication_date(raw_html, undateable_url) assert pub_date is None pub_date = dates.guess_publication_date( diff --git a/mcmetadata/test/test_extract.py b/mcmetadata/test/test_extract.py index e34dad2..77166a7 100644 --- a/mcmetadata/test/test_extract.py +++ b/mcmetadata/test/test_extract.py @@ -2,10 +2,9 @@ import unittest import mcmetadata -from mcmetadata.test import read_fixture +from mcmetadata.test import mock_fetch from .. import content, extract -from ..exceptions import BadContentError class TestExtract(unittest.TestCase): @@ -26,7 +25,7 @@ def test_shortened(self): def test_homepage(self): url = "https://web.archive.org/web/" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) results = extract(url, raw_html) assert "is_homepage" in results assert results["is_homepage"] is True @@ -36,7 +35,7 @@ def test_no_date(self): url = ( "https://web.archive.org/web/20260116030752/https://somervilleunitedfc.org/" ) - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) results = extract(url, raw_html) assert "publication_date" in results assert results["publication_date"] is None @@ -45,7 +44,7 @@ def test_no_date(self): def test_observers(self): test_url = "https://observers.france24.com/en/20190826-mexico-african-migrants-trapped-protest-journey" - raw_html = read_fixture(test_url) + raw_html, _ = mock_fetch(test_url) results = extract(test_url, raw_html) assert "publication_date" in results assert results["publication_date"] == dt.datetime(2019, 8, 27, 0, 0) @@ -68,30 +67,16 @@ def test_observers(self): == "https://observers.france24.com/en/20190826-mexico-african-migrants-trapped-protest-journey" ) - def test_archived_url(self): - # properly handle pages at web archives (via memento headers) - test_url = "https://web.archive.org/web/20181210092018/https://www.nytimes.com/interactive/2018/12/10/business/location-data-privacy-apps.html" - results = extract( - test_url - ) # need to fetch original (not cached) to get headers that will be processed to set URL correctly - assert "canonical_domain" in results - assert results["canonical_domain"] == "nytimes.com" - assert "original_url" in results - assert ( - results["url"] - == "https://www.nytimes.com/interactive/2018/12/10/business/location-data-privacy-apps.html" - ) - def test_language(self): url = "https://web.archive.org/web/https://www.mk.co.kr/news/society/view/2020/07/693939/" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) results = extract(url, raw_html) assert "language" in results assert results["language"] == "ko" def test_regionalized_language(self): url = "https://web.archive.org/web/http://entretenimento.uol.com.br/noticias/redacao/2019/08/25/sem-feige-sem-stark-o-sera-do-homem-aranha-longe-do-mcu.htm" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) results = extract(url, raw_html) assert "pt" == results["language"] assert "pt-br" == results["full_language"] @@ -111,7 +96,7 @@ def test_redirected_url(self): def test_basic(self): url = "https://www.indiatimes.com/news/india/75th-independence-day-india-august-15-576959.html" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) results = extract(url, raw_html) assert url == results["original_url"] assert url == results["url"] @@ -122,7 +107,7 @@ def test_basic(self): def test_other_metadata(self): url = "https://www.indiatimes.com/news/india/75th-independence-day-india-august-15-576959.html" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) results = extract(url, raw_html, include_other_metadata=True) assert url == results["original_url"] assert url == results["url"] @@ -140,20 +125,12 @@ def test_whitespace_removal(self): previous_min_content_length = content.MINIMUM_CONTENT_LENGTH content.MINIMUM_CONTENT_LENGTH = 10 url = "https://observador.vsports.pt/embd/75404/m/9812/obsrv/53a58b677b53143428e47d43d5887139?autostart=false" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) results = extract(url, raw_html) # the point here is that it removes all pre and post whitespace - tons of junk assert len(results["text_content"]) == 110 content.MINIMUM_CONTENT_LENGTH = previous_min_content_length - def test_memento_without_original_url(self): - try: - url = "https://web.archive.org/web/20210412063445id_/https://ehp.niehs.nih.gov/action/doUpdateAlertSettings?action=addJournal&journalCode=ehp&referrer=/action/doSearch?ContribAuthorRaw=Davis%2C+Jacquelyn&ContentItemType=research-article&startPage=&ContribRaw=Martin%2C+Denny" - _ = extract(url, include_other_metadata=True) - assert False - except BadContentError: - assert True - def test_overrides(self): url = "https://www.indiatimes.com/news/india/75th-independence-day-india-august-15-576959.html" overrides = dict( @@ -164,7 +141,7 @@ def test_overrides(self): publication_date=dt.date(2023, 1, 1), ) # validate not the same as overrides - html_content = read_fixture(url) + html_content, _ = mock_fetch(url) results = extract(url, html_content) assert results["text_content"] != overrides["text_content"] assert results["article_title"] != overrides["article_title"] @@ -181,11 +158,12 @@ def test_overrides(self): def test_default_title(self): # throws too short error if no default url = "https://web.archive.org/web/20111013162600id_/http://www.azftf.gov/(F(r8GSI1MAawoG8fkwp0vWYNSTuweOi8-9wgJOr4j83rTcpZDuFOV5E2PG737tNitGhzYAsUmVcwVEcgwKEtYFADTmzsQMJto9bZTOzDBHUGRpirFPIt4osB08CAslzBk-ih5ATrsM-P7DRxDwcNdmfB4jU1Y1))/WhatWeDo/Volunteer/Pages/default.aspx" - results = extract(url) + raw_html, _ = mock_fetch(url) + results = extract(url, raw_html) assert results["article_title"] is None # verify throws too short error defaults = dict(article_title="This is a title") - results = extract(url, defaults=defaults) + results = extract(url, raw_html, defaults=defaults) assert results["article_title"] == defaults["article_title"] def test_default_pub_date(self): @@ -214,7 +192,7 @@ class TestStats(unittest.TestCase): def test_reset(self): url = "https://web.archive.org/web/http://entretenimento.uol.com.br/noticias/redacao/2019/08/25/sem-feige-sem-stark-o-sera-do-homem-aranha-longe-do-mcu.htm" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) _ = extract(url, raw_html) assert mcmetadata.stats.get("total") > 0 mcmetadata.reset_stats() @@ -222,7 +200,7 @@ def test_reset(self): def test_total_works(self): url = "https://web.archive.org/web/http://entretenimento.uol.com.br/noticias/redacao/2019/08/25/sem-feige-sem-stark-o-sera-do-homem-aranha-longe-do-mcu.htm" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) _ = extract(url, raw_html) assert mcmetadata.stats.get("total") > 0 for s in mcmetadata.STAT_NAMES: @@ -235,7 +213,7 @@ def test_passed_in_accumulator(self): mcmetadata.reset_stats() local_stats = {s: 0 for s in mcmetadata.STAT_NAMES} url = "https://web.archive.org/web/http://entretenimento.uol.com.br/noticias/redacao/2019/08/25/sem-feige-sem-stark-o-sera-do-homem-aranha-longe-do-mcu.htm" - raw_html = read_fixture(url) + raw_html, _ = mock_fetch(url) _ = extract(url, raw_html, stats_accumulator=local_stats) for s in mcmetadata.STAT_NAMES: # verify global counter didn't count assert s in mcmetadata.stats @@ -246,4 +224,4 @@ def test_passed_in_accumulator(self): if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() diff --git a/mcmetadata/test/test_languages.py b/mcmetadata/test/test_languages.py index e6e0837..9f2a7bb 100644 --- a/mcmetadata/test/test_languages.py +++ b/mcmetadata/test/test_languages.py @@ -1,13 +1,13 @@ import unittest from .. import content, languages -from . import read_fixture +from . import mock_fetch class TestLanguageFromText(unittest.TestCase): def _fetch_and_validate(self, url: str, expected_language_code: str): - html_text = read_fixture(url) + html_text, _ = mock_fetch(url) article = content.from_html(url, html_text) lang_code = languages._from_text(article["text"]) assert lang_code == expected_language_code @@ -51,7 +51,7 @@ def test_language_without_region(self): class TestLanguageFromHtml(unittest.TestCase): def _fetch_and_validate(self, url: str, expected_language_code: str): - html_text = read_fixture(url) + html_text, _ = mock_fetch(url) article = content.from_html(url, html_text) lang_code = languages.from_html(html_text, article["text"]) assert lang_code == expected_language_code diff --git a/mcmetadata/test/test_titles.py b/mcmetadata/test/test_titles.py index ffa7301..4b122af 100644 --- a/mcmetadata/test/test_titles.py +++ b/mcmetadata/test/test_titles.py @@ -4,13 +4,13 @@ import pytest from .. import titles, webpages -from . import read_fixture +from . import mock_fetch class TestTitle(unittest.TestCase): def _fetch_and_validate(self, url: str, expected_title: Optional[str]): - html_text = read_fixture(url) + html_text, _ = mock_fetch(url) assert titles.from_html(html_text) == expected_title def test_only_h1(self): diff --git a/mcmetadata/test/test_webpages.py b/mcmetadata/test/test_webpages.py index 9e5ac07..195a524 100644 --- a/mcmetadata/test/test_webpages.py +++ b/mcmetadata/test/test_webpages.py @@ -5,6 +5,8 @@ import requests from .. import webpages +from ..exceptions import BadContentError +from . import mock_fetch class TestFetch(unittest.TestCase): @@ -21,17 +23,6 @@ def test_regular_fetch(self): assert "Boston Globe" in html assert response.encoding == "utf-8" - def test_non_utf8_encoding_fix(self): - url = "https://web.archive.org/web/https://www.mk.co.kr/news/society/view/2020/07/693939/" - html, response = webpages.fetch(url, fix_encoding=False) - assert response.status_code == 200 - assert response.encoding == "ISO-8859-1" - assert response.apparent_encoding == "EUC-KR" - html, response = webpages.fetch(url, fix_encoding=True) - assert response.status_code == 200 - assert response.encoding == "EUC-KR" - assert response.apparent_encoding == "EUC-KR" - def test_bad_domain(self): try: url = "https://123_NO_DOIMAN" @@ -48,6 +39,35 @@ def test_bad_response(self): except RuntimeError: assert True + class TestFinalUrl(unittest.TestCase): + + def test_archived_url(self): + # properly handle pages at web archives (via memento headers) + original_url = "https://www.nytimes.com/interactive/2018/12/10/business/location-data-privacy-apps.html" + test_url = "https://web.archive.org/web/20181210092018/https://www.nytimes.com/interactive/2018/12/10/business/location-data-privacy-apps.html" + headers = { + "memento-datetime": "Mon, 10 Dec 2018 09:20:18 GMT", + "link": f'<{original_url}>; rel="original"', + } + raw_html, response = mock_fetch(test_url, headers=headers) + final_url = webpages.final_url(response) + # Did it correctly pull the original URL out of the link header because the memento-datetime was there? + assert final_url == original_url + + def test_memento_without_original_url(self): + try: + test_url = "https://web.archive.org/web/20210412063445id_/https://ehp.niehs.nih.gov/action/doUpdateAlertSettings?action=addJournal&journalCode=ehp&referrer=/action/doSearch?ContribAuthorRaw=Davis%2C+Jacquelyn&ContentItemType=research-article&startPage=&ContribRaw=Martin%2C+Denny" + headers = { + "memento-datetime": "Mon, 10 Dec 2018 09:20:18 GMT", + } + raw_html, response = mock_fetch(test_url, headers=headers) + _ = webpages.final_url( + response + ) # should raise BadContentError, since archived but we can't tell from where + assert False + except BadContentError: + assert True + if __name__ == "__main__": unittest.main() diff --git a/pyproject.toml b/pyproject.toml index edb0c35..e4f5d81 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,7 +59,7 @@ dev = [ "types-requests==2.32.*", "black==24.4.*" ] -test = [ "pytest", "parameterized" ] +test = [ "pytest", "parameterized", "requests-mock" ] [project.urls] "Homepage" = "https://mediacloud.org" From c73bf29277e8c0b1156537cfca59a93be3bbb984 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:45:27 +0200 Subject: [PATCH 18/31] script to compare results in old warc vs. current code #105 --- .gitignore | 2 + pyproject.toml | 3 +- scripts/evaluate-against-warc.py | 96 ++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 scripts/evaluate-against-warc.py diff --git a/.gitignore b/.gitignore index e0e5386..9400e94 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ build dist *.egg-info venv/ +*.warc.gz +*.warc diff --git a/pyproject.toml b/pyproject.toml index e4f5d81..d3d55db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,7 +57,8 @@ dev = [ "isort==5.12.*", "types-urllib3==1.26.*", "types-requests==2.32.*", - "black==24.4.*" + "black==24.4.*", + "warcio==1.8.*" ] test = [ "pytest", "parameterized", "requests-mock" ] diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py new file mode 100644 index 0000000..3dabe96 --- /dev/null +++ b/scripts/evaluate-against-warc.py @@ -0,0 +1,96 @@ +import argparse +import gzip +import json +import logging +import os +import shutil +import tempfile + +from warcio.archiveiterator import ArchiveIterator + +import mcmetadata + +logger = logging.getLogger(__name__) + + +def compare(record_id: str, url: str, html: str, old_metadata: dict): + new_metadata = mcmetadata.extract(url, html) + new_metadata["publication_date"] = str(new_metadata["publication_date"].date()) + for key, value in old_metadata.items(): + if key == "parsed_date": + continue + if key not in new_metadata: + raise Exception(f"{record_id}: '{key}' not in metadata") + if new_metadata[key] != value: + raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") + + +def evaluate(warc_file_path: str, max_records: int = None) -> None: + logger.info("Evaluating WARC file: %s", warc_file_path) + fail_count = 0 + pass_count = 0 + with open(warc_file_path, "rb") as stream: + url = None + record_id = None + html_content = None + records_compared = 0 + for record in ArchiveIterator(stream): + if record.rec_type == "response" and record_id is None: + url = record.rec_headers.get_header("WARC-Target-URI") + record_id = record.rec_headers.get_header("WARC-Record-ID") + html_content = ( + record.content_stream().read().decode("utf-8", errors="replace") + ) + elif record.rec_type == "metadata" and record_id is not None: + if record.rec_headers.get_header("WARC-Refers-To") == record_id: + metadata_record = record + # logger.info(f"found match for {record_id}") + metadata_content = ( + metadata_record.content_stream() + .read() + .decode("utf-8", errors="replace") + ) + metadata = json.loads(metadata_content) + try: + compare( + record_id, url, html_content, metadata["content_metadata"] + ) + pass_count += 1 + except Exception as e: + logger.error(f"{record_id}: fail {e}") + fail_count += 1 + logger.info(f"{record_id}: pass") + records_compared += 1 + url = None + record_id = None + html_content = None + if max_records and (records_compared >= max_records): + logger.info(f"stopping at {max_records} records") + return + logger.info( + f"Done with {fail_count} fails and {pass_count} passes ({fail_count + pass_count} total stories)" + ) + + +if __name__ == "__main__": + logging.basicConfig(level=logging.INFO) + parser = argparse.ArgumentParser( + description="Evaluate metadata extraction against a WARC file." + ) + parser.add_argument( + "warc_path", help="Path to the WARC file (handles .warc.gz or .warc)" + ) + args = parser.parse_args() + + warc_path = args.warc_path + if warc_path.endswith(".gz"): + with tempfile.NamedTemporaryFile(suffix=".warc", delete=False) as tmp: + tmp_path = tmp.name + try: + with gzip.open(warc_path, "rb") as gz_in, open(tmp_path, "wb") as tmp_out: + shutil.copyfileobj(gz_in, tmp_out) + evaluate(tmp_path) + finally: + os.unlink(tmp_path) + else: + evaluate(warc_path) From 310862543db4a3ebb45aa55af80bc121cd7080a5 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:55:49 +0200 Subject: [PATCH 19/31] compare extracted text as length, with wiggle room --- scripts/evaluate-against-warc.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 3dabe96..96311b6 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -21,8 +21,16 @@ def compare(record_id: str, url: str, html: str, old_metadata: dict): continue if key not in new_metadata: raise Exception(f"{record_id}: '{key}' not in metadata") - if new_metadata[key] != value: - raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") + if key == "text_content": + if abs(len(new_metadata[key]) - len(value)) > ( + len(new_metadata[key]) * 0.2 + ): + raise Exception( + f"{record_id}: '{key}' {len(value)} != {len(new_metadata[key])}" + ) + else: + if new_metadata[key] != value: + raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") def evaluate(warc_file_path: str, max_records: int = None) -> None: @@ -59,7 +67,7 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: except Exception as e: logger.error(f"{record_id}: fail {e}") fail_count += 1 - logger.info(f"{record_id}: pass") + # logger.info(f"{record_id}: pass") records_compared += 1 url = None record_id = None From e06e45e4a3b3353ed2515c41c593cc005e26cc4f Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:56:10 +0200 Subject: [PATCH 20/31] upgrade htmldate and dateparser (no change in evaluation results) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d3d55db..f56e69e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ classifiers = [ ] dependencies = [ # for date guessing - "htmldate==1.8.*", "dateparser==1.2.*", + "htmldate==1.10.*", "dateparser==1.4.*", # for domain name and URL extraction "tldextract==5.1.*", "url-normalize==1.4.*", From 3b27b7aa4123cf529e6d4b1357caf0e12a2ca119 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 17:03:16 +0200 Subject: [PATCH 21/31] upgrade URL dependencies (to test) --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f56e69e..19bd159 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,8 +28,8 @@ dependencies = [ # for date guessing "htmldate==1.10.*", "dateparser==1.4.*", # for domain name and URL extraction - "tldextract==5.1.*", - "url-normalize==1.4.*", + "tldextract==5.3.*", + "url-normalize==3.0.*", "furl==2.1.*", # for language detection "py3langid==0.2.*", From cfef8b35a6d7854c6c8806584dc6f1b90232f9b5 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 11:45:49 +0200 Subject: [PATCH 22/31] update py3langid; fix test_utm_removal unit tests to match bugfix in url_normalize --- mcmetadata/test/test_urls.py | 4 ++-- pyproject.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mcmetadata/test/test_urls.py b/mcmetadata/test/test_urls.py index db62bff..1ab9c36 100644 --- a/mcmetadata/test/test_urls.py +++ b/mcmetadata/test/test_urls.py @@ -129,8 +129,8 @@ def test_utm_removal(self): url = "http://fake.com/article?foo=123&baz=321" normalized_url = urls.normalize_url(url) assert ( - normalized_url == "http://fake.com/article?baz=321&foo=123" - ) # they get ordered + normalized_url == url + ) # params are not reordered (order matters apparently) url = "http://fake.com/article?utm_foo=123&baz=321" normalized_url = urls.normalize_url(url) assert normalized_url == "http://fake.com/article?baz=321" diff --git a/pyproject.toml b/pyproject.toml index 19bd159..1972a7b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,7 +32,7 @@ dependencies = [ "url-normalize==3.0.*", "furl==2.1.*", # for language detection - "py3langid==0.2.*", + "py3langid==0.3.*", # various content extractors we try to use "newspaper3k==0.2.*", "goose3==3.1.*", From 5a5b112af2fe77fe37a07458fb50d67d852d59b1 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 11:50:34 +0200 Subject: [PATCH 23/31] update faust-cchardet; all unit tests and evaluates same against old warc --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 1972a7b..09464fb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,7 +42,7 @@ dependencies = [ "boilerpy3==1.0.*", # support "requests", # leave un-versioned so dependencies can sort of which version is best - "faust-cchardet==2.1.*", # BeautifulSoup4 speedup + "faust-cchardet==3.1.*", # BeautifulSoup4 speedup "surt==0.3.1" ] From bb75ba8b2dbe7f646d6a3d8d33a6e1ca5b5fb4dd Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 12:40:12 +0200 Subject: [PATCH 24/31] add extractor check, also AI-suggested fix to let me run tests in PyCharm --- mcmetadata/test/test_content.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mcmetadata/test/test_content.py b/mcmetadata/test/test_content.py index 7fbf5e4..c113d73 100644 --- a/mcmetadata/test/test_content.py +++ b/mcmetadata/test/test_content.py @@ -16,7 +16,10 @@ class TestContentMetadata(unittest.TestCase): def test_top_image(self): html_text, _ = mock_fetch(self.URL) meta = content.from_html(self.URL, html_text) + meta = content.from_html(self.URL, html_text, False) + assert meta["extraction_method"] == "trafilatura" assert meta["top_image_url"] == self.EXPRECTED_IMG_URL + meta = content.from_html(self.URL, html_text, True) class TestContentParsers(unittest.TestCase): From 836f84edb3b0292992114200a23a74f8a184d364 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:26:20 +0200 Subject: [PATCH 25/31] upgrade to latest trafilatura for testing --- mcmetadata/content.py | 24 +++++++++++------------- pyproject.toml | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/mcmetadata/content.py b/mcmetadata/content.py index c4af6da..725a56e 100644 --- a/mcmetadata/content.py +++ b/mcmetadata/content.py @@ -79,8 +79,8 @@ def from_html(url: str, html_text: str, include_metadata: bool = False) -> Dict: method_success_stats[extractor.content["extraction_method"]] += 1 extractor.content["text"] = extractor.content["text"].strip() return extractor.content - except BadContentError as e: - raise e + except BadContentError as bce: + raise bce except Exception: # if the extractor fails for any reason, just continue on to the next one pass @@ -177,29 +177,27 @@ class TrafilaturaExtractor(AbstractExtractor): def extract(self, url: str, html_text: str, include_metadata: bool = False): results = trafilatura.bare_extraction( html_text, - only_with_metadata=include_metadata, url=url, include_images=include_metadata, + with_metadata=True, # important to get title, authors, url, etc. ) image_urls = [] if include_metadata: # pull out the images embedded in the markdown - for match in markdown_img_path_pattern.finditer(results["text"]): + for match in markdown_img_path_pattern.finditer(results.text): image_urls.append(match.group(1)) # remove the image links from the full text - text = markdown_img_path_pattern.sub("", results["text"]) + text = markdown_img_path_pattern.sub("", results.text) else: - text = results["text"] + text = results.text self.content = { "url": url, "text": text, - "title": results["title"], - "canonical_url": results[ - "url" - ], # Warning: This will not work with Trafilatura v1.11.* and later - "potential_publish_date": dateparser.parse(results["date"]), - "top_image_url": image_urls[0] if len(image_urls) > 0 else results["image"], - "authors": results["author"].split(",") if results["author"] else None, + "title": results.title, + "canonical_url": results.url, + "potential_publish_date": dateparser.parse(results.date), + "top_image_url": image_urls[0] if len(image_urls) > 0 else results.image, + "authors": results.author.split(",") if results.author else None, "extraction_method": METHOD_TRAFILATURA, } diff --git a/pyproject.toml b/pyproject.toml index 09464fb..0aeedbe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,9 +36,9 @@ dependencies = [ # various content extractors we try to use "newspaper3k==0.2.*", "goose3==3.1.*", - "BeautifulSoup4==4.12.*", + "BeautifulSoup4==4.15.*", "readability-lxml==0.8.*", - "trafilatura==1.8.*", # must stay below v1.11.* to allow easy extraction of canonical_url + "trafilatura==2.2.*", "boilerpy3==1.0.*", # support "requests", # leave un-versioned so dependencies can sort of which version is best From 2fd0d00acf92a88b8e48fcdadbd39711925e9e45 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:26:51 +0200 Subject: [PATCH 26/31] minor unit test fix (hand-checked and its ok) --- mcmetadata/test/test_extract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcmetadata/test/test_extract.py b/mcmetadata/test/test_extract.py index 77166a7..4a781f4 100644 --- a/mcmetadata/test/test_extract.py +++ b/mcmetadata/test/test_extract.py @@ -49,7 +49,7 @@ def test_observers(self): assert "publication_date" in results assert results["publication_date"] == dt.datetime(2019, 8, 27, 0, 0) assert "text_content" in results - assert len(results["text_content"]) > 7000 + assert len(results["text_content"]) > 6900 assert "text_extraction_method" in results assert results["text_extraction_method"] == content.METHOD_TRAFILATURA assert "canonical_domain" in results From 062e9c57c59df857d728a568da8641ea62f17417 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:27:30 +0200 Subject: [PATCH 27/31] better checking and logging around eval of text_content changes --- scripts/evaluate-against-warc.py | 55 +++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 96311b6..3dbceed 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -12,25 +12,51 @@ logger = logging.getLogger(__name__) +CONTENT_LENGTH_CONCERN_THRESHOLD = 0.2 + + +def _concerning_content_diff(old_content, new_content): + return abs(len(old_content) - len(new_content)) > ( + len(new_content) * CONTENT_LENGTH_CONCERN_THRESHOLD + ) + def compare(record_id: str, url: str, html: str, old_metadata: dict): new_metadata = mcmetadata.extract(url, html) new_metadata["publication_date"] = str(new_metadata["publication_date"].date()) for key, value in old_metadata.items(): - if key == "parsed_date": + # not a concern if now trafilatura works (over readability) prior, unless content is too different + if key == "text_extraction_method": + if new_metadata[key] != value: + # logger.info(f"Old content length = {len(value)}; new length = {len(new_metadata[key])}") + if _concerning_content_diff(value, new_metadata[key]): + raise Exception( + f"{record_id}: '{key}' {value} != {new_metadata[key]}" + ) + else: + continue + # the parsed_date isn't content that is static + elif key == "parsed_date": continue - if key not in new_metadata: + # is there some data missing in the new extraction results? + elif key not in new_metadata: raise Exception(f"{record_id}: '{key}' not in metadata") - if key == "text_content": - if abs(len(new_metadata[key]) - len(value)) > ( - len(new_metadata[key]) * 0.2 - ): + # check the test within a reasonable threshold + elif key == "text_content": + # logger.info(f"{record_id}: was {len(value)} now {len(new_metadata[key])}") + if _concerning_content_diff(value, new_metadata[key]): + if new_metadata["language"] == "en": + continue raise Exception( f"{record_id}: '{key}' {len(value)} != {len(new_metadata[key])}" ) + # just check if the value has changed from what we got before else: + # logger.info(f" {key}: {value} -> {new_metadata[key]}") if new_metadata[key] != value: - raise Exception(f"{record_id}: '{key}' {value} != {new_metadata[key]}") + raise Exception( + f"{record_id}: '{key}' was {value} != now {new_metadata[key]}" + ) def evaluate(warc_file_path: str, max_records: int = None) -> None: @@ -83,10 +109,17 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: if __name__ == "__main__": logging.basicConfig(level=logging.INFO) parser = argparse.ArgumentParser( - description="Evaluate metadata extraction against a WARC file." + description="Evaluate metadata extraction against a WARC file.", + ) + parser.add_argument( + "warc_path", + help="Path to the WARC file (handles .warc.gz or .warc)", ) parser.add_argument( - "warc_path", help="Path to the WARC file (handles .warc.gz or .warc)" + "max_records", + help="A number indicating how many records to check at most", + default=None, + type=int, ) args = parser.parse_args() @@ -97,8 +130,8 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: try: with gzip.open(warc_path, "rb") as gz_in, open(tmp_path, "wb") as tmp_out: shutil.copyfileobj(gz_in, tmp_out) - evaluate(tmp_path) + evaluate(tmp_path, args.max_records) finally: os.unlink(tmp_path) else: - evaluate(warc_path) + evaluate(warc_path, args.max_records) From 71b1ee79f178f529d68bb2d20445c49aceb8ace8 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:32:54 +0200 Subject: [PATCH 28/31] remove redundant id from error log --- scripts/evaluate-against-warc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 3dbceed..10e38b6 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -91,7 +91,7 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: ) pass_count += 1 except Exception as e: - logger.error(f"{record_id}: fail {e}") + logger.error(f"fail {e}") fail_count += 1 # logger.info(f"{record_id}: pass") records_compared += 1 From e5e42e0328251bcbc83badb99c324d82b7bf3b74 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Mon, 24 Aug 2026 15:09:16 +0200 Subject: [PATCH 29/31] tldextract upgrade: from registered_domain -> top_domain_under_public_suffix --- mcmetadata/urls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mcmetadata/urls.py b/mcmetadata/urls.py index 3926363..5aef9e3 100644 --- a/mcmetadata/urls.py +++ b/mcmetadata/urls.py @@ -76,11 +76,11 @@ def canonical_domain(raw_url: str) -> str: candidate_domain = ( parsed_domain.subdomain.lower() + "." - + parsed_domain.registered_domain.lower() + + parsed_domain.top_domain_under_public_suffix.lower() ) else: # default to "registered domain" the URL is attributed to - candidate_domain = parsed_domain.registered_domain.lower() + candidate_domain = parsed_domain.top_domain_under_public_suffix.lower() # also handle amp URLs smartly if "cdn.ampproject.org" in candidate_domain: From d543b8702856e56ab98eb6a16c4f53791585af6f Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Thu, 20 Aug 2026 16:45:27 +0200 Subject: [PATCH 30/31] script to compare results in old warc vs. current code #105 --- scripts/evaluate-against-warc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index 10e38b6..b1e4b9d 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -134,4 +134,4 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: finally: os.unlink(tmp_path) else: - evaluate(warc_path, args.max_records) + evaluate(warc_path, args.max_records) \ No newline at end of file From 3b88aa61d078f929cf71fbf8845f8a1211de94d1 Mon Sep 17 00:00:00 2001 From: Rahul Bhargava Date: Fri, 21 Aug 2026 14:27:30 +0200 Subject: [PATCH 31/31] better checking and logging around eval of text_content changes --- scripts/evaluate-against-warc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/evaluate-against-warc.py b/scripts/evaluate-against-warc.py index b1e4b9d..10e38b6 100644 --- a/scripts/evaluate-against-warc.py +++ b/scripts/evaluate-against-warc.py @@ -134,4 +134,4 @@ def evaluate(warc_file_path: str, max_records: int = None) -> None: finally: os.unlink(tmp_path) else: - evaluate(warc_path, args.max_records) \ No newline at end of file + evaluate(warc_path, args.max_records)