From 0a941acc43bcbc9d1f4e11f9591bff4039e45558 Mon Sep 17 00:00:00 2001
From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com>
Date: Tue, 11 Aug 2026 20:16:11 +0200
Subject: [PATCH] refactor: split the site validator into its two passes

validate_site reached complexity 43 doing three things at once. The
per-page checks and the link resolution are now separate functions and
the caller keeps only the cross-page duplicate check, which cannot run
until every page has been seen.

The first attempt left the link pass calling a closure that had moved,
and running it against the real site did not catch that: no page there
has a broken link, so the error path never ran. The unit test covering
a deliberately broken fragment did.
---
 scripts/validate_site.py | 63 +++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/scripts/validate_site.py b/scripts/validate_site.py
index aaeef026..982eda74 100644
--- a/scripts/validate_site.py
+++ b/scripts/validate_site.py
@@ -147,20 +147,17 @@ def _local_target(
     return candidates, unquote(parsed.fragment)
 
 
-def validate_site(site: Path, config_path: Path) -> list[str]:
-    site = site.resolve()
-    if not site.is_dir():
-        return [f"site directory does not exist: {site}"]
+def _check_pages(
+    pages: dict, site: Path, titles: dict, descriptions: dict
+) -> list[str]:
+    """Every per-page check: title, description, links, JSON-LD.
 
-    base_path = _base_path(config_path)
-    html_paths = sorted(site.rglob("*.html"))
-    if not html_paths:
-        return [f"no HTML pages found in {site}"]
-
-    pages = {path.resolve(): _parse_page(path) for path in html_paths}
+    Split out of validate_site, which reached complexity 43. The
+    cross-page duplicate check stays with the caller, since it can only
+    run once every page has been seen. *titles* and *descriptions* are
+    filled here for it.
+    """
     issues: list[str] = []
-    titles: dict[str, list[Path]] = defaultdict(list)
-    descriptions: dict[str, list[Path]] = defaultdict(list)
 
     def report(path: Path, message: str) -> None:
         issues.append(f"{path.relative_to(site)}: {message}")
@@ -207,11 +204,15 @@ def validate_site(site: Path, config_path: Path) -> list[str]:
             if not isinstance(payload, dict) or payload.get("@context") != "https://schema.org":
                 report(path, "JSON-LD is not a schema.org object")
 
-    for label, values in (("title", titles), ("description", descriptions)):
-        for value, paths in values.items():
-            if len(paths) > 1:
-                rendered = ", ".join(str(path.relative_to(site)) for path in paths[:5])
-                issues.append(f"duplicate {label} {value!r}: {rendered}")
+    return issues
+
+
+def _check_links(pages: dict, site: Path, base_path: str) -> list[str]:
+    """Every local link and fragment resolves to something on disk."""
+    issues: list[str] = []
+
+    def report(path: Path, message: str) -> None:
+        issues.append(f"{path.relative_to(site)}: {message}")
 
     for path, page in pages.items():
         for href in page.links:
@@ -234,6 +235,34 @@ def validate_site(site: Path, config_path: Path) -> list[str]:
     return issues
 
 
+def validate_site(site: Path, config_path: Path) -> list[str]:
+    site = site.resolve()
+    if not site.is_dir():
+        return [f"site directory does not exist: {site}"]
+
+    base_path = _base_path(config_path)
+    html_paths = sorted(site.rglob("*.html"))
+    if not html_paths:
+        return [f"no HTML pages found in {site}"]
+
+    pages = {path.resolve(): _parse_page(path) for path in html_paths}
+    issues: list[str] = []
+    titles: dict[str, list[Path]] = defaultdict(list)
+    descriptions: dict[str, list[Path]] = defaultdict(list)
+
+    issues.extend(_check_pages(pages, site, titles, descriptions))
+
+    for label, values in (("title", titles), ("description", descriptions)):
+        for value, paths in values.items():
+            if len(paths) > 1:
+                rendered = ", ".join(str(path.relative_to(site)) for path in paths[:5])
+                issues.append(f"duplicate {label} {value!r}: {rendered}")
+
+    issues.extend(_check_links(pages, site, base_path))
+
+    return issues
+
+
 def main() -> None:
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument("--site-dir", default="site")