Tests for check_broken_links and freeze(), explainer structure check (3 issues)

.pre-commit-config.yaml

@@ -48,6 +48,13 @@ repos:
         pass_filenames: false
         files: '^(explainers/.*\.md|README\.md|CONTRIBUTORS\.md|METRICS\.md|ROADMAP\.md)$'
 
+      - id: check-explainer-structure
+        name: check explainers have the required sections
+        entry: python3 scripts/check_explainer_structure.py
+        language: system
+        pass_filenames: false
+        files: '^(explainers/.*\.md|scripts/check_explainer_structure\.py)$'
+
       - id: pytest
         name: pytest (fast suite, skips slow benchmark tests)
         entry: python3 -m pytest tests/ -q -m "not slow"

CHANGELOG.md

@@ -21,6 +21,8 @@ All notable changes to Fair Code are documented here, newest first.
 
 ### Added
 
+- `scripts/check_explainer_structure.py` (#1118), run by `make lint` and pre-commit: every explainer must have the sections CONTRIBUTING.md lists (accepting the usual heading variants, in order), a `python` block, two Further Reading entries and the footer. Ten pages with existing gaps are listed in `KNOWN_EXCEPTIONS`, and an exception that is no longer needed fails the check so the list only shrinks.
+- Tests for `scripts/check_broken_links.py` (#1109, 96% coverage) and for the whole `freeze()` flow in `scripts/freeze_paper_results.py` (#1111, 99% coverage), run against temporary directories.
 - `scripts/check_traction.py` (#1097), run by `make lint`: fails, naming the files and values, when the stars, forks, contributors, watching, social reach, countries or audits figures differ between the README, ROADMAP and METRICS, or when the README's "Show N completed items" differs from its `- [x]` lines. It compares the files with each other, so it works offline.
 - `scripts/diff_results.py` marks rows measured on different test sets (#1095): each row has a `sample` field (`same`, `different` or `unknown`, with `sample_sizes` when different), different rows are marked `~` in the text output and counted in `--summary`.
 - CONTRIBUTING.md lists the prerequisites for `make check` and a "Checks you may meet" table saying what each failing check means and how to fix it (#989).

CONTRIBUTING.md

@@ -87,6 +87,7 @@ Prerequisites for the full `make check`: Python 3.10 or newer, Node 18 or newer
 | `check_broken_links.py` | A relative link in the docs points at a missing file or heading | Fix the path or heading |
 | `check_issue_refs.py` | A `#NNN` in ROADMAP, CHANGELOG, CONTRIBUTORS or METRICS is not a real issue or PR | Correct the number |
 | `check_explainer_count.py` | An "N explainers" figure, or an explainer listing in the README or `llms.txt`, is stale | `make fix-explainer-count` (a missing listing has to be added by hand) |
+| `check_explainer_structure.py` | An explainer is missing a required section (definition, why it matters, concrete example, detection code, limitations, related concepts, further reading), has no `python` block, has fewer than two Further Reading entries, or lacks the footer line | Add the section (the check accepts the usual heading variants, e.g. "Why This Matters"); pages with a known gap are listed in `KNOWN_EXCEPTIONS` in the script, and an exception is removed when its page is fixed |
 | `check_traction.py` | Stars, forks, contributors, watching, social reach, countries or audits differ between README, ROADMAP and METRICS, or the README's "Show N completed items" differs from its `- [x]` lines | Edit the figure in the file(s) the message names so they agree; the README's "External Contributors" is one below the total |
 | `key-fold-check` | The JavaScript case-folding table `KEY_FOLD` no longer matches Python | `make key-fold`, then commit `assets/profiler-engine.js` |
 | A test that runs a README command | Every `faircode ...` line in the README, CONTRIBUTING and SPEC code blocks is checked against the CLI's argument parser; one of them uses a flag or value that no longer exists | Fix the example or the CLI |

Makefile

@@ -45,11 +45,12 @@ favicons:  ## Regenerate favicon.ico/PNGs and apple-touch-icon.png from logo.svg
 fix-explainer-count:  ## Correct stale "N explainers" mentions in README/CONTRIBUTORS/METRICS/ROADMAP
 	$(PY) scripts/check_explainer_count.py --fix
 
-lint:  ## Enforce the em-dash-free rule + check for broken doc links + issue numbers in docs (needs gh, skipped offline) + explainer counts and listings + traction figures agreeing across README/ROADMAP/METRICS + ruff (mirrors the lint workflow)
+lint:  ## Enforce the em-dash-free rule + check for broken doc links + issue numbers in docs (needs gh, skipped offline) + explainer counts and listings + explainer structure + traction figures agreeing across README/ROADMAP/METRICS + ruff (mirrors the lint workflow)
 	$(PY) scripts/check_em_dash.py
 	$(PY) scripts/check_broken_links.py
 	$(PY) scripts/check_issue_refs.py
 	$(PY) scripts/check_explainer_count.py
+	$(PY) scripts/check_explainer_structure.py
 	$(PY) scripts/check_traction.py
 	ruff check faircode scripts tests
 

scripts/check_explainer_structure.py

@@ -0,0 +1,163 @@
+#!/usr/bin/env python3
+"""Checks that every explainers/*.md has the structure CONTRIBUTING.md section 8 asks for (#1118).
+
+A good explainer has, in this order: a definition, why it matters, a concrete example, detection
+code, limitations, related concepts and further reading. It also has at least one `python` fenced
+block, at least two Further Reading entries, and the project footer line. Headings are worded
+differently from page to page ("Why It Matters" / "Why This Matters", "Limitations" /
+"Limitations and Trade-offs"), so each required section lists the heading patterns it accepts.
+
+    python3 scripts/check_explainer_structure.py
+
+Exit code: 0 = every explainer passes, 1 = at least one problem (each is printed with the file and
+what is missing).
+
+KNOWN_EXCEPTIONS lists the gaps that already exist, per page, so the check can land before the
+pages are fixed. An exception is removed by fixing the page; the check also fails when an exception
+is no longer needed, so the list can only shrink.
+"""
+from __future__ import annotations
+
+import re
+import sys
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parent.parent
+EXPLAINERS_DIR = ROOT / "explainers"
+
+# (name, regex matched against the lower-cased "## " heading text), in the required order.
+SECTIONS = [
+    ("definition", r"definition|core concept"),
+    ("why it matters", r"why (it|this) matters"),
+    ("concrete example", r"concrete example|real-world (proof|case)|real implementation|tabular proof|the benchmark"),
+    ("detection code", r"detect|how to measure"),
+    ("limitations", r"limitations"),
+    ("related concepts", r"related (concepts|projects)"),
+    ("further reading", r"further reading"),
+]
+SECTION_NAMES = [name for name, _ in SECTIONS]
+
+MIN_FURTHER_READING_ENTRIES = 2
+FOOTER_PREFIX = "*Part of [The Fair Code Project]"
+
+# slug -> what that page is allowed to lack for now. "order" excuses a section order that differs
+# from CONTRIBUTING.md. Remove an entry when the page is fixed (the check fails on a stale one).
+KNOWN_EXCEPTIONS: dict[str, set[str]] = {
+    "automation-bias": {"limitations"},
+    "neural-networks": {"limitations", "detection code"},
+    "proxy-variables": {"limitations"},
+    "sampling-bias": {"limitations", "related concepts"},
+    "differential-privacy": {"concrete example"},
+    "disparate-impact": {"detection code"},
+    "fairness-metric-conflicts": {"detection code"},
+    "shap-values": {"detection code"},
+    "how-ai-detects-patterns": {"order"},
+    "predictive-parity": {"order"},
+}
+
+FENCE_RE = re.compile(r"^\s*(```|~~~)")
+H2_RE = re.compile(r"^##\s+(.*?)\s*#*\s*$")
+
+
+def _headings_and_flags(text: str):
+    """The (line_index, lower-cased text) of every top-level "## " heading outside code fences,
+    plus whether a python fence exists."""
+    headings, in_fence, has_python = [], False, False
+    for i, line in enumerate(text.splitlines()):
+        fence = FENCE_RE.match(line)
+        if fence:
+            if not in_fence and line.strip().lower().startswith(("```python", "```py", "~~~python")):
+                has_python = True
+            in_fence = not in_fence
+            continue
+        if in_fence:
+            continue
+        m = H2_RE.match(line)
+        if m:
+            headings.append((i, m.group(1).strip().lower()))
+    return headings, has_python
+
+
+def _further_reading_entries(text: str) -> int:
+    """List items under the last "Further Reading" heading, up to the next heading or rule."""
+    lines = text.splitlines()
+    start = None
+    for i, line in enumerate(lines):
+        m = H2_RE.match(line)
+        if m and "further reading" in m.group(1).lower():
+            start = i
+    if start is None:
+        return 0
+    count = 0
+    for line in lines[start + 1:]:
+        if H2_RE.match(line):
+            break
+        if re.match(r"^\s*(?:[-*]|\d+\.)\s+\S", line):
+            count += 1
+    return count
+
+
+def problems_in(text: str) -> set[str]:
+    """What is wrong with one explainer, as a set of short names (a section name, "order",
+    "python block", "further reading entries" or "footer")."""
+    headings, has_python = _headings_and_flags(text)
+    found: dict[str, int] = {}
+    for name, pattern in SECTIONS:
+        for line_no, heading in headings:
+            if re.search(pattern, heading):
+                found[name] = line_no
+                break
+    problems = {name for name in SECTION_NAMES if name not in found}
+    in_order = [found[name] for name in SECTION_NAMES if name in found]
+    if in_order != sorted(in_order):
+        problems.add("order")
+    if not has_python:
+        problems.add("python block")
+    if "further reading" in found and _further_reading_entries(text) < MIN_FURTHER_READING_ENTRIES:
+        problems.add("further reading entries")
+    last = next((line for line in reversed(text.splitlines()) if line.strip()), "")
+    if not last.startswith(FOOTER_PREFIX):
+        problems.add("footer")
+    return problems
+
+
+def check(directory: Path | None = None, exceptions: dict[str, set[str]] | None = None) -> list[str]:
+    """One message per problem across every `*.md` in `directory` (empty when all is well)."""
+    directory = EXPLAINERS_DIR if directory is None else directory
+    exceptions = KNOWN_EXCEPTIONS if exceptions is None else exceptions
+    messages = []
+    seen = set()
+    for path in sorted(Path(directory).glob("*.md")):
+        slug = path.stem
+        seen.add(slug)
+        found = problems_in(path.read_text(encoding="utf-8"))
+        allowed = exceptions.get(slug, set())
+        new = found - allowed
+        if new:
+            messages.append(f"{path.name}: missing or wrong: {', '.join(sorted(new))}")
+        stale = allowed - found
+        if stale:
+            messages.append(
+                f"{path.name}: no longer needs the exception for {', '.join(sorted(stale))}; "
+                "remove it from KNOWN_EXCEPTIONS in scripts/check_explainer_structure.py"
+            )
+    for slug in sorted(set(exceptions) - seen):
+        messages.append(f"KNOWN_EXCEPTIONS names {slug!r}, but explainers/{slug}.md does not exist")
+    return messages
+
+
+def main() -> int:
+    messages = check()
+    if messages:
+        print("Explainer structure problems (see CONTRIBUTING.md, 'A good explainer should include'):",
+              file=sys.stderr)
+        for message in messages:
+            print(f"  - {message}", file=sys.stderr)
+        return 1
+    count = len(list(EXPLAINERS_DIR.glob("*.md")))
+    print(f"OK: {count} explainers have the required sections, a python block, further reading and the footer.")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/test_check_broken_links.py

@@ -0,0 +1,169 @@
+"""scripts/check_broken_links.py catches broken in-page anchors and relative links (#1109)."""
+
+import importlib
+
+import pytest
+
+cbl = importlib.import_module("scripts.check_broken_links")
+
+
+@pytest.fixture
+def repo(tmp_path, monkeypatch):
+    """A temporary repo: write(rel_path, text) creates a file that git 'tracks'."""
+    tracked = set()
+
+    def write(rel, text=""):
+        path = tmp_path / rel
+        path.parent.mkdir(parents=True, exist_ok=True)
+        path.write_text(text, encoding="utf-8")
+        tracked.add(rel)
+        return path
+
+    monkeypatch.setattr(cbl, "ROOT", tmp_path)
+    monkeypatch.setattr(cbl, "_tracked_paths", lambda: set(tracked))
+    monkeypatch.setattr(
+        cbl, "_tracked_markdown_files",
+        lambda: [tmp_path / rel for rel in sorted(tracked) if rel.endswith(".md")],
+    )
+    write.root = tmp_path
+    return write
+
+
+def _run(capsys):
+    code = cbl.main()
+    return code, capsys.readouterr().out
+
+
+def test_clean_repo_passes_and_reports_the_counts(repo, capsys):
+    repo("README.md", "# Title\n\nSee [guide](docs/guide.md) and [top](#title).\n")
+    repo("docs/guide.md", "# Guide\n")
+    code, out = _run(capsys)
+    assert code == 0
+    assert out.startswith("OK: 2 internal links checked across 2 markdown files")
+
+
+def test_a_relative_link_to_a_missing_file_fails_with_file_line_and_target(repo, capsys):
+    repo("README.md", "# Title\n\nSee [guide](docs/missing.md).\n")
+    code, out = _run(capsys)
+    assert code == 1
+    assert "Broken internal links found:" in out
+    assert "README.md:3: [docs/missing.md]" in out
+    assert "does not exist" in out
+
+
+def test_an_in_page_anchor_must_match_a_heading(repo, capsys):
+    repo("README.md", "# Title\n\n[ok](#title) and [bad](#nope)\n")
+    code, out = _run(capsys)
+    assert code == 1
+    assert "[#nope] - no matching heading on this page" in out
+    assert "[#title]" not in out
+
+
+def test_an_anchor_in_another_file_must_match_a_heading_there(repo, capsys):
+    repo("README.md", "[ok](docs/guide.md#setup) [bad](docs/guide.md#teardown)\n")
+    repo("docs/guide.md", "# Guide\n\n## Setup\n")
+    code, out = _run(capsys)
+    assert code == 1
+    assert "no heading 'teardown' in docs/guide.md" in out
+    assert "#setup" not in out
+
+
+def test_a_link_to_a_directory_is_fine(repo, capsys):
+    repo("README.md", "[folder](docs/)\n")
+    repo("docs/guide.md", "# Guide\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_external_links_are_ignored(repo, capsys):
+    repo("README.md", "[a](https://example.invalid/x) [b](mailto:a@b.c) [c](ftp://h/f)\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_images_are_not_checked_as_links(repo, capsys):
+    repo("README.md", "![logo](assets/missing.png)\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_links_inside_fenced_code_and_inline_code_are_ignored(repo, capsys):
+    repo("README.md", "# T\n\n```md\n[bad](nope.md) [bad2](#nowhere)\n```\n\nUse `[x](gone.md)` here.\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_headings_inside_fences_do_not_count_as_anchors(repo, capsys):
+    repo("README.md", "# T\n\n```md\n# Fake\n```\n\n[x](#fake)\n")
+    code, out = _run(capsys)
+    assert code == 1
+    assert "[#fake]" in out
+
+
+def test_a_link_title_is_stripped_from_the_target(repo, capsys):
+    repo("README.md", '[guide](docs/guide.md "The guide")\n')
+    repo("docs/guide.md", "# Guide\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_percent_encoded_paths_are_decoded(repo, capsys):
+    repo("README.md", "[a](docs/my%20file.md)\n")
+    repo("docs/my file.md", "# X\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_blob_permalink_style_paths_are_skipped(repo, capsys):
+    repo(".github/TEMPLATE.md", "[x](../blob/main/README.md)\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_a_file_on_disk_that_git_does_not_track_is_a_broken_link(repo, capsys):
+    repo("README.md", "[x](docs/guide.md)\n")
+    (repo.root / "docs").mkdir()
+    (repo.root / "docs" / "guide.md").write_text("# G\n", encoding="utf-8")   # not tracked
+    code, out = _run(capsys)
+    assert code == 1
+    assert "docs/guide.md" in out
+
+
+def test_a_link_that_escapes_the_repo_falls_back_to_the_filesystem(repo, capsys):
+    repo("README.md", "[x](../outside.md)\n")
+    code, out = _run(capsys)
+    assert code == 1
+    assert "../outside.md" in out
+
+
+def test_every_bad_link_is_reported_not_just_the_first(repo, capsys):
+    repo("README.md", "[a](one.md)\n[b](two.md)\n")
+    code, out = _run(capsys)
+    assert code == 1
+    assert "README.md:1: [one.md]" in out and "README.md:2: [two.md]" in out
+
+
+@pytest.mark.parametrize("heading,slug", [
+    ("Plain Title", "plain-title"),
+    ("COMPAS - Criminal Justice Bias", "compas--criminal-justice-bias"),
+    ("Auto-Detection", "auto-detection"),
+    ("What is `K`?", "what-is-k"),
+    ("**Bold** and [link](x.md)", "bold-and-link"),
+])
+def test_slugify_matches_github_style_anchors(heading, slug):
+    assert cbl._slugify(heading) == slug
+
+
+def test_duplicate_headings_get_numbered_anchors(repo, capsys):
+    repo("README.md", "# Notes\n\n# Notes\n\n[second](#notes-1)\n")
+    code, _ = _run(capsys)
+    assert code == 0
+
+
+def test_generated_explainer_mirror_is_excluded_by_default():
+    assert cbl.ALLOW_PREFIXES == ("faircode/_explainers/",)
+
+
+def test_the_real_repo_has_no_broken_links(capsys):
+    assert cbl.main() == 0
+    capsys.readouterr()