refactor(llm,rex): normalize FR text at capture; index-time strip becomes guard (refs #617)

dev/scripts/fetch_fr_attachments.py

@@ -4,6 +4,13 @@ For each rule item with a ``document_number``, queries the Federal
 Register API for the PDF and raw text URLs, downloads both, and
 attaches them to the item in the bib store.
 
+The FR TXT response is HTML-wrapped (``<html><pre>...``); when it looks
+that way, the raw bytes are saved alongside as ``<doc>.html`` and the
+``.txt`` is normalized in place with ``rex.frtext.clean_fr_text`` before
+being attached — the bib attachment always points at the cleaned
+``.txt``. See ``dev/scripts/migrate_fr_txt.py`` for the one-off
+migration over files downloaded before this normalization existed.
+
 Usage::
 
     uv run python dev/scripts/fetch_fr_attachments.py            # fetch all
@@ -22,6 +29,7 @@ from pathlib import Path
 import httpx
 
 from conf import connect
+from rex.frtext import normalize_txt_file
 
 FR_API = "https://www.federalregister.gov/api/v1/documents"
 
@@ -166,6 +174,7 @@ def main() -> int:
         if not has_txt and txt_url:
             txt_path = download_dir / txt_name
             if _download_file(txt_url, txt_path):
+                normalize_txt_file(txt_path)
                 store.attach_file(rule.key, txt_path, title=f"FR {doc_num} (TXT)")
                 print(f"    TXT: {txt_path.stat().st_size:,} bytes")
                 stats["downloaded"] += 1

dev/scripts/migrate_fr_txt.py

@@ -0,0 +1,77 @@
+"""One-off migration: normalize already-downloaded FR ``.txt`` files.
+
+Follow-up to issue #617. Before this, ``dev/scripts/fetch_fr_attachments.py``
+stored the Federal Register API's raw HTML-wrapped TXT response
+directly as the ``.txt`` attachment, so cleanup (strip the
+``<html><pre>`` wrapper, inline markup, entities, control chars) ran at
+every index run instead of once at capture time.
+
+This walks ``data/fr_downloads/*.txt`` and, for every file that still
+looks HTML-wrapped, saves the raw bytes to a sibling ``.html`` (skipped
+if it already exists) and rewrites the ``.txt`` in place with
+``rex.frtext.clean_fr_text`` — the same normalization
+``fetch_fr_attachments.py`` now applies immediately after each fresh
+download.
+
+Usage::
+
+    uv run python dev/scripts/migrate_fr_txt.py --dry-run   # preview
+    uv run python dev/scripts/migrate_fr_txt.py              # migrate
+"""
+
+from __future__ import annotations
+
+import argparse
+import sys
+from pathlib import Path
+
+from rex.frtext import looks_html_wrapped, normalize_txt_file
+
+DEFAULT_DIR = Path("data/fr_downloads")
+
+
+def find_candidates(root: Path) -> list[Path]:
+    """``*.txt`` files directly under *root* that still look HTML-wrapped."""
+    out = []
+    for path in sorted(root.glob("*.txt")):
+        raw = path.read_text(encoding="utf-8", errors="replace")
+        if looks_html_wrapped(raw):
+            out.append(path)
+    return out
+
+
+def migrate_one(path: Path, *, dry_run: bool = False) -> dict:
+    """Normalize one file. Returns a report row with before/after sizes."""
+    before = path.stat().st_size
+    if dry_run:
+        return {"path": path, "before": before, "after": before, "changed": False}
+    normalize_txt_file(path)
+    after = path.stat().st_size
+    return {"path": path, "before": before, "after": after, "changed": True}
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument(
+        "--dir", default=str(DEFAULT_DIR), help="Directory of *.txt to scan."
+    )
+    parser.add_argument("--dry-run", action="store_true")
+    args = parser.parse_args(argv)
+
+    root = Path(args.dir)
+    candidates = find_candidates(root)
+    if not candidates:
+        print(f"No HTML-wrapped .txt files found under {root}")
+        return 0
+
+    verb = "would rewrite" if args.dry_run else "rewrote"
+    print(f"{len(candidates)} HTML-wrapped file(s) under {root}:")
+    for path in candidates:
+        row = migrate_one(path, dry_run=args.dry_run)
+        print(f"  [{verb}] {path.name}: {row['before']:,} -> {row['after']:,} bytes")
+
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

src/llm/source.py

@@ -15,8 +15,6 @@ so parsing the comment id is the reliable path to a comment's docket.
 
 from __future__ import annotations
 
-import html
-import re
 from pathlib import Path
 from typing import Iterator
 
@@ -125,45 +123,18 @@ def _attachment_text(store: Store, item_key: str) -> str:
     return "\n\n".join(parts)
 
 
-_TAG_RE = re.compile(r"</?[A-Za-z][^>]*>")
-
-
-def _strip_html_wrapper(text: str) -> str:
-    """Drop the FR ``.txt`` attachment's HTML wrapper and inline markup.
-
-    FR TXT attachments are HTML-wrapped: ``<html><head>...<body><pre>``
-    then the plain text, closed with ``</pre></body></html>``. Strip
-    everything through the first ``<pre>`` and any trailing close tags;
-    text without that wrapper passes through unchanged.
-
-    The FR text itself also carries inline HTML — e.g. Cloudflare's
-    obfuscated ``<a href="/cdn-cgi/l/email-protection#...">`` mailto
-    anchors and page-break anchors. Those are stripped with a
-    ``</?[A-Za-z][^>]*>`` regex (this is a controlled, known corpus —
-    not attacker-supplied HTML — so no HTML-parsing dependency is
-    needed), keeping the anchor's inner text. The tag must start with a
-    letter (optionally preceded by ``/``) so free-text comparison
-    fragments like ``p<0.05`` or ``x>2`` are never mistaken for markup.
-    HTML entities (``&amp;`` etc.) are then unescaped. Plain-text
-    markers like ``[[Page NNNNN]]`` contain no angle brackets and pass
-    through untouched.
-    """
-    idx = text.find("<pre>")
-    if idx == -1:
-        body = text
-    else:
-        body = text[idx + len("<pre>") :]
-        stripped = body.rstrip()
-        for suffix in ("</pre></body></html>", "</pre>"):
-            if stripped.endswith(suffix):
-                body = stripped[: -len(suffix)]
-                break
-    body = _TAG_RE.sub("", body)
-    return html.unescape(body)
-
-
 def _rule_text(store: Store, item_key: str) -> str:
-    """TXT attachment (HTML wrapper stripped) preferred; else PDF-extract."""
+    """TXT attachment (HTML wrapper stripped) preferred; else PDF-extract.
+
+    The FR ``.txt`` attachment is normalized once at capture time (see
+    ``dev/scripts/fetch_fr_attachments.py`` and the one-off
+    ``dev/scripts/migrate_fr_txt.py`` migration) via the shared
+    ``rex.frtext.clean_fr_text`` cleaner. It's applied again here as a
+    cheap, idempotent guard — not a load-bearing transform — so
+    not-yet-migrated or freshly-downloaded files still come out clean.
+    """
+    from rex.frtext import clean_fr_text
+
     rows = (
         store._con()
         .execute(
@@ -176,9 +147,7 @@ def _rule_text(store: Store, item_key: str) -> str:
     for (storage_path,) in rows:
         path = Path(storage_path)
         if path.suffix.lower() == ".txt" and path.exists():
-            return _strip_html_wrapper(
-                path.read_text(encoding="utf-8", errors="replace")
-            )
+            return clean_fr_text(path.read_text(encoding="utf-8", errors="replace"))
     return _attachment_text(store, item_key)
 
 

src/rex/frtext.py

@@ -0,0 +1,90 @@
+"""Shared cleaner for Federal Register full-text attachments.
+
+FR ``.txt`` downloads are HTML-wrapped: ``<html><head>...<body><pre>``
+then the plain text, closed with ``</pre></body></html>``. The text
+inside also carries inline HTML — e.g. Cloudflare's obfuscated
+``<a href="/cdn-cgi/l/email-protection#...">`` mailto anchors and
+page-break anchors — plus HTML entities and stray control characters
+(NUL, form-feeds) that Postgres text columns reject.
+
+Historically this cleanup ran at *index* time, once per doc per
+(re)index run (see ``llm.source._rule_text``). It now runs once at
+*capture* time (``dev/scripts/fetch_fr_attachments.py`` and the
+one-off ``dev/scripts/migrate_fr_txt.py`` migration for already
+downloaded files), with the index-time call kept as a cheap,
+idempotent guard rather than a load-bearing transform.
+"""
+
+from __future__ import annotations
+
+import html
+import re
+from pathlib import Path
+
+# The tag must start with a letter (optionally preceded by ``/``) so
+# free-text comparison fragments like ``p<0.05`` or ``x>2`` are never
+# mistaken for markup. This is a controlled, known corpus — not
+# attacker-supplied HTML — so no HTML-parsing dependency is needed.
+_TAG_RE = re.compile(r"</?[A-Za-z][^>]*>")
+
+# C0 control chars except tab/newline/carriage-return. Extracted text
+# carries NUL (0x00) and form-feeds that Postgres text columns reject
+# ("cannot contain NUL bytes") and that add no semantic value.
+_CONTROL_RE = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f]")
+
+
+def clean_fr_text(raw: str) -> str:
+    """Strip the FR ``.txt`` HTML wrapper, inline markup, entities and
+    control chars from *raw*.
+
+    Idempotent: ``clean_fr_text(clean_fr_text(x)) == clean_fr_text(x)``.
+    Plain-text markers like ``[[Page NNNNN]]`` contain no angle
+    brackets and pass through untouched, as does text without an HTML
+    wrapper.
+    """
+    idx = raw.find("<pre>")
+    if idx == -1:
+        body = raw
+    else:
+        body = raw[idx + len("<pre>") :]
+        stripped = body.rstrip()
+        for suffix in ("</pre></body></html>", "</pre>"):
+            if stripped.endswith(suffix):
+                body = stripped[: -len(suffix)]
+                break
+    body = _TAG_RE.sub("", body)
+    body = html.unescape(body)
+    return _CONTROL_RE.sub("", body)
+
+
+def looks_html_wrapped(raw: str) -> bool:
+    """True when *raw* looks like the FR API's HTML-wrapped TXT response.
+
+    Checked against the first 200 chars so a plain-text document that
+    happens to mention ``<html`` deep inside its body isn't mistaken
+    for a wrapped one.
+    """
+    return raw[:200].lstrip().lower().startswith("<html")
+
+
+def normalize_txt_file(txt_path: Path, *, html_path: Path | None = None) -> bool:
+    """Split an HTML-wrapped ``.txt`` into a raw ``.html`` sibling + a
+    cleaned ``.txt``. No-op when *txt_path* doesn't look HTML-wrapped.
+
+    Used both right after a fresh FR TXT download
+    (``dev/scripts/fetch_fr_attachments.py``) and by the one-off
+    ``dev/scripts/migrate_fr_txt.py`` migration over already-downloaded
+    files — same shape, same rule: save the raw bytes once (skipped if
+    the ``.html`` sibling already exists), then overwrite ``.txt`` with
+    ``clean_fr_text``.
+
+    Returns True when *txt_path* was rewritten.
+    """
+    raw = txt_path.read_text(encoding="utf-8", errors="replace")
+    if not looks_html_wrapped(raw):
+        return False
+    dest_html = html_path if html_path is not None else txt_path.with_suffix(".html")
+    if not dest_html.exists():
+        dest_html.write_bytes(txt_path.read_bytes())
+    txt_path.write_text(clean_fr_text(raw), encoding="utf-8")
+    return True

tests/dev/test_migrate_fr_txt.py

@@ -0,0 +1,97 @@
+"""Tests for dev/scripts/migrate_fr_txt.py — one-off migration that
+normalizes already-downloaded FR ``.txt`` files (refs #617).
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import sys
+from pathlib import Path
+
+# Load the script as a module — it lives outside the src/ tree.
+_SCRIPT = Path(__file__).resolve().parents[2] / "dev" / "scripts" / "migrate_fr_txt.py"
+_spec = importlib.util.spec_from_file_location("_migrate_fr_txt", _SCRIPT)
+assert _spec and _spec.loader
+_migrate_fr_txt = importlib.util.module_from_spec(_spec)
+sys.modules["_migrate_fr_txt"] = _migrate_fr_txt
+_spec.loader.exec_module(_migrate_fr_txt)
+
+WRAPPED = (
+    "<html><head><title>x</title></head><body><pre>\n"
+    "The Secretary proposes to amend 42 CFR part 414.\n"
+    "</pre></body></html>"
+)
+
+
+class TestFindCandidates:
+    def test_finds_only_html_wrapped_files(self, tmp_path):
+        (tmp_path / "wrapped.txt").write_text(WRAPPED)
+        (tmp_path / "plain.txt").write_text("Already plain.")
+        (tmp_path / "ignored.pdf").write_bytes(b"%PDF-1.4")
+
+        found = _migrate_fr_txt.find_candidates(tmp_path)
+
+        assert [p.name for p in found] == ["wrapped.txt"]
+
+    def test_empty_dir_yields_nothing(self, tmp_path):
+        assert _migrate_fr_txt.find_candidates(tmp_path) == []
+
+
+class TestMigrateOne:
+    def test_dry_run_leaves_file_untouched(self, tmp_path):
+        txt = tmp_path / "2025-13271.txt"
+        txt.write_text(WRAPPED)
+
+        row = _migrate_fr_txt.migrate_one(txt, dry_run=True)
+
+        assert row["changed"] is False
+        assert txt.read_text() == WRAPPED
+        assert not (tmp_path / "2025-13271.html").exists()
+
+    def test_real_run_writes_html_sibling_and_cleans_txt(self, tmp_path):
+        txt = tmp_path / "2025-13271.txt"
+        txt.write_text(WRAPPED)
+
+        row = _migrate_fr_txt.migrate_one(txt, dry_run=False)
+
+        assert row["changed"] is True
+        assert row["before"] == len(WRAPPED.encode())
+        html_sibling = tmp_path / "2025-13271.html"
+        assert html_sibling.read_text() == WRAPPED
+        cleaned = txt.read_text()
+        assert "<html>" not in cleaned
+        assert "The Secretary proposes to amend 42 CFR part 414." in cleaned
+        assert row["after"] == len(cleaned.encode())
+
+
+class TestMain:
+    def test_dry_run_reports_without_writing(self, tmp_path, capsys):
+        txt = tmp_path / "2025-13271.txt"
+        txt.write_text(WRAPPED)
+
+        rc = _migrate_fr_txt.main(["--dir", str(tmp_path), "--dry-run"])
+
+        assert rc == 0
+        assert txt.read_text() == WRAPPED
+        out = capsys.readouterr().out
+        assert "2025-13271.txt" in out
+
+    def test_real_run_migrates_matching_files(self, tmp_path):
+        txt = tmp_path / "2025-13271.txt"
+        txt.write_text(WRAPPED)
+        (tmp_path / "plain.txt").write_text("Nothing to do here.")
+
+        rc = _migrate_fr_txt.main(["--dir", str(tmp_path)])
+
+        assert rc == 0
+        assert (tmp_path / "2025-13271.html").exists()
+        assert "<html>" not in txt.read_text()
+        assert not (tmp_path / "plain.html").exists()
+
+    def test_no_candidates_is_a_clean_noop(self, tmp_path, capsys):
+        (tmp_path / "plain.txt").write_text("Nothing wrapped here.")
+
+        rc = _migrate_fr_txt.main(["--dir", str(tmp_path)])
+
+        assert rc == 0
+        assert "No HTML-wrapped" in capsys.readouterr().out

tests/rex/test_frtext.py

@@ -0,0 +1,152 @@
+"""rex.frtext — shared FR rule-text cleaner (capture-time normalization).
+
+Mirrors the scenarios that used to live against
+``llm.source._strip_html_wrapper`` (see tests/llm/test_source.py) since
+that's the logic this module now owns.
+"""
+
+from rex.frtext import clean_fr_text, looks_html_wrapped, normalize_txt_file
+
+
+class TestHtmlWrapper:
+    def test_strips_html_pre_wrapper(self):
+        raw = (
+            "<html><head><title>x</title></head><body><pre>\n"
+            "The Secretary proposes to amend 42 CFR part 414.\n"
+            "</pre></body></html>"
+        )
+        cleaned = clean_fr_text(raw)
+        assert "The Secretary proposes to amend 42 CFR part 414." in cleaned
+        assert "<html>" not in cleaned
+        assert "<pre>" not in cleaned
+
+    def test_passthrough_when_no_wrapper(self):
+        raw = "Plain text, no wrapper at all."
+        assert clean_fr_text(raw) == raw
+
+
+class TestInlineMarkupAndEntities:
+    def test_inline_html_and_entities_stripped(self):
+        raw = (
+            "<html><head><title>x</title></head><body><pre>\n"
+            "[[Page 12345]]\n"
+            'Contact us at <a href="/cdn-cgi/l/email-protection#x">'
+            "someone</a> regarding CMS &amp; PFS.\n"
+            "A finding was significant (p<0.05 and >2 cm).\n"
+            "</pre></body></html>"
+        )
+        cleaned = clean_fr_text(raw)
+        assert "<a href=" not in cleaned
+        assert "</a>" not in cleaned
+        assert "someone" in cleaned
+        assert "CMS & PFS" in cleaned
+        assert "[[Page 12345]]" in cleaned
+        assert "p<0.05 and >2 cm" in cleaned
+
+
+class TestControlChars:
+    def test_strips_nul_and_c0_control_chars(self):
+        raw = "line one\x00\x0bline two\x1f end"
+        cleaned = clean_fr_text(raw)
+        assert "\x00" not in cleaned
+        assert "\x0b" not in cleaned
+        assert "\x1f" not in cleaned
+        assert "line one" in cleaned
+        assert "line two" in cleaned
+
+    def test_preserves_tab_newline_cr(self):
+        raw = "a\tb\nc\rd"
+        cleaned = clean_fr_text(raw)
+        assert cleaned == "a\tb\nc\rd"
+
+
+class TestIdempotency:
+    def test_wrapped_text_is_idempotent(self):
+        raw = (
+            "<html><head><title>x</title></head><body><pre>\n"
+            "[[Page 12345]]\n"
+            'Contact <a href="/cdn-cgi/l/email-protection#x">someone</a> '
+            "re CMS &amp; PFS.\n"
+            "A finding (p<0.05 and >2 cm).\x00\x0b\n"
+            "</pre></body></html>"
+        )
+        once = clean_fr_text(raw)
+        twice = clean_fr_text(once)
+        assert once == twice
+
+    def test_plain_text_is_idempotent(self):
+        raw = "Plain text with p<0.05 and >2 cm, no markup."
+        assert clean_fr_text(raw) == clean_fr_text(clean_fr_text(raw))
+
+    def test_empty_string(self):
+        assert clean_fr_text("") == ""
+
+
+class TestLooksHtmlWrapped:
+    def test_true_for_html_wrapped(self):
+        assert looks_html_wrapped("<html><head></head><body><pre>x</pre></body></html>")
+
+    def test_true_case_insensitive_and_leading_whitespace(self):
+        assert looks_html_wrapped("  \n<HTML><body><pre>x</pre></body></html>")
+
+    def test_false_for_plain_text(self):
+        assert not looks_html_wrapped("Plain text, no wrapper at all.")
+
+    def test_false_when_html_appears_deep_in_body(self):
+        raw = ("filler " * 100) + "<html>should not count, too deep"
+        assert not looks_html_wrapped(raw)
+
+
+class TestNormalizeTxtFile:
+    def test_rewrites_wrapped_file_and_saves_raw_html(self, tmp_path):
+        txt = tmp_path / "2025-13271.txt"
+        raw = (
+            "<html><head><title>x</title></head><body><pre>\n"
+            "The Secretary proposes to amend 42 CFR part 414.\n"
+            "</pre></body></html>"
+        )
+        txt.write_text(raw)
+
+        changed = normalize_txt_file(txt)
+
+        assert changed is True
+        html_sibling = tmp_path / "2025-13271.html"
+        assert html_sibling.exists()
+        assert html_sibling.read_text() == raw
+        cleaned = txt.read_text()
+        assert "<html>" not in cleaned
+        assert "The Secretary proposes to amend 42 CFR part 414." in cleaned
+
+    def test_noop_for_plain_text(self, tmp_path):
+        txt = tmp_path / "plain.txt"
+        txt.write_text("Already plain text.")
+
+        changed = normalize_txt_file(txt)
+
+        assert changed is False
+        assert not (tmp_path / "plain.html").exists()
+        assert txt.read_text() == "Already plain text."
+
+    def test_does_not_clobber_existing_html_sibling(self, tmp_path):
+        txt = tmp_path / "2025-13271.txt"
+        txt.write_text("<html><body><pre>new wrapped text</pre></body></html>")
+        html_sibling = tmp_path / "2025-13271.html"
+        html_sibling.write_text("original raw capture, do not overwrite")
+
+        normalize_txt_file(txt)
+
+        assert html_sibling.read_text() == "original raw capture, do not overwrite"
+
+    def test_idempotent_on_repeat_calls(self, tmp_path):
+        txt = tmp_path / "2025-13271.txt"
+        txt.write_text(
+            "<html><body><pre>[[Page 1]] p<0.05 and >2 cm</pre></body></html>"
+        )
+
+        normalize_txt_file(txt)
+        first = txt.read_text()
+        # Second call sees already-cleaned plain text: no wrapper, no-op.
+        changed_again = normalize_txt_file(txt)
+
+        assert changed_again is False
+        assert txt.read_text() == first