mdscrub: decode character references, keep block boundaries, narrow the fence

parity/KNOWN.md

@@ -18,25 +18,6 @@ affected and how often it occurs in practice.
 
 ---
 
-## mdscrub — a line-break tag becomes a line break, not nothing
-
-**Python:** `strip_residual_html` removes every inline tag, `<br>` included,
-so `the<br>northern end` becomes `thenorthern end`.
-
-**Rust:** `<br>`, `<br/>` and `<br />` become a newline; every other tag is
-still removed.
-
-**Why this is acceptable:** the welded word is not a formatting difference,
-it is a word that does not exist. It cannot be searched for, it embeds as a
-token sequence no query will produce, and it is invisible in a result because
-the surrounding sentence reads normally. The divergence only affects
-documents that use `<br>` at all — mostly verse, addresses and tables of
-contents converted out of HTML — and within those, only the joins.
-
-**Measured:** `parity/cases/mdscrub/breaks.md`; both answers are committed as
-`golden/mdscrub/breaks.md` and `golden/mdscrub/breaks.rust.md`. Three welds
-in eight lines.
-
 ## mdscrub — un-paginated text is not given a page 1 marker
 
 **Python:** `_normalize_marker_pages` ends by wrapping any text it found no

parity/README.md

@@ -23,9 +23,14 @@ can be re-run live, which catches a golden file drifting away from a Python
 implementation that has moved on:
 
 ```sh
-CATENA_PARITY_PYTHON=/path/to/venv/bin/python cargo test -p parity --features python
+CATENA_PARITY_PYTHON=/path/to/venv/bin/python \
+CATENA_PARITY_PYTHONPATH=/path/to/book-pipeline/src \
+    cargo test -p parity --features python
 ```
 
+`SOURCE.md` records which commit of that source the golden files came from.
+Pointing the oracles at an older checkout will fail, and should.
+
 ## Regenerating
 
 Golden files are never regenerated by a test — a golden file that rewrites
@@ -33,7 +38,9 @@ itself when it fails is a test that cannot fail. Regenerate deliberately,
 read the diff, and commit it with the reason:
 
 ```sh
-CATENA_PARITY_PYTHON=/path/to/venv/bin/python cargo run -p parity --bin regen
+CATENA_PARITY_PYTHON=/path/to/venv/bin/python \
+CATENA_PARITY_PYTHONPATH=/path/to/book-pipeline/src \
+    cargo run -p parity --bin regen
 ```
 
 ## Fixtures

parity/SOURCE.md

@@ -0,0 +1,19 @@
+# What the golden files were generated from
+
+The oracles import the Python these tools were ported from. The golden files
+in this directory are the answers that Python gave, at one commit:
+
+| Source | Commit |
+|---|---|
+| `book-pipeline` | `7499e3257572968b437ecfdc79feccc91b96585b` |
+
+Regenerating against a different commit will move the goldens. That is the
+point — the diff is the review. Record the new commit here in the same change.
+
+The oracles find the source through two environment variables, because where
+a checkout lives is a fact about one machine and must not be compiled in:
+
+```sh
+CATENA_PARITY_PYTHON=/path/to/venv/bin/python
+CATENA_PARITY_PYTHONPATH=/path/to/book-pipeline/src
+```

parity/golden/mdscrub/breaks.md

@@ -1,5 +1,5 @@
-The address was set with explicit line breaks:Twelve Mill LaneThe
-Valleyand the county beyond.
+The address was set with explicit line breaks: Twelve Mill Lane The
+Valley and the county beyond.
 
 A second paragraph, where the break sits mid sentence — the mill stood at
-thenorthern end of the valley, where the fall was steepest.
+the northern end of the valley, where the fall was steepest.

parity/golden/mdscrub/breaks.rust.md

@@ -1,9 +0,0 @@
-The address was set with explicit line breaks:
-Twelve Mill Lane
-The
-Valley
-and the county beyond.
-
-A second paragraph, where the break sits mid sentence — the mill stood at
-the
-northern end of the valley, where the fall was steepest.

parity/oracle/unescape.py

@@ -0,0 +1,23 @@
+"""Decode HTML character references the way Python does, one line at a time.
+
+Each line of stdin is one case; each line of stdout is that case decoded and
+JSON-encoded, because a decoded reference can itself be a newline.
+
+    python3 oracle/unescape.py < cases/unescape/entities.txt
+"""
+
+import html
+import json
+import sys
+
+
+def main() -> int:
+    text = sys.stdin.buffer.read().decode("utf-8")
+    out = sys.stdout
+    for line in text.split("\n"):
+        out.write(json.dumps(html.unescape(line), ensure_ascii=False) + "\n")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

parity/src/bin/regen.rs

@@ -19,8 +19,9 @@ type Written = Result<Vec<String>, String>;
 type Job = (&'static str, fn() -> Written);
 
 fn main() -> ExitCode {
-    let jobs: [Job; 3] = [
+    let jobs: [Job; 4] = [
         ("tokens", tokens),
+        ("unescape", unescape),
         ("chunkmd", chunkmd),
         ("mdscrub", mdscrub),
     ];
@@ -54,6 +55,18 @@ fn tokens() -> Written {
     Ok(vec![path.display().to_string()])
 }
 
+fn unescape() -> Written {
+    let dir = parity::golden("unescape");
+    std::fs::create_dir_all(&dir).map_err(|e| e.to_string())?;
+
+    let cases = parity::dir().join("cases/unescape/entities.txt");
+    let text = std::fs::read_to_string(&cases).map_err(|e| e.to_string())?;
+    let decoded = parity::run_oracle("unescape.py", &text)?;
+    let path = dir.join("entities.jsonl");
+    std::fs::write(&path, decoded).map_err(|e| e.to_string())?;
+    Ok(vec![path.display().to_string()])
+}
+
 fn mdscrub() -> Written {
     let dir = parity::golden("mdscrub");
     std::fs::create_dir_all(&dir).map_err(|e| e.to_string())?;
@@ -66,15 +79,6 @@ fn mdscrub() -> Written {
         let path = dir.join(format!("{name}.md"));
         std::fs::write(&path, cleaned).map_err(|e| e.to_string())?;
         written.push(path.display().to_string());
-
-        // Where the Rust deliberately differs, its own answer is committed
-        // too, so the difference is visible in a review rather than only in
-        // a test failure. See parity/KNOWN.md.
-        if parity::DIVERGENT_MDSCRUB.contains(&name.as_str()) {
-            let path = dir.join(format!("{name}.rust.md"));
-            std::fs::write(&path, ::mdscrub::scrub(&markdown)).map_err(|e| e.to_string())?;
-            written.push(path.display().to_string());
-        }
     }
     Ok(written)
 }

parity/src/lib.rs

@@ -83,10 +83,6 @@ pub fn run_oracle_with(script: &str, args: &[String], input: &str) -> Result<Str
         .map_err(|e| format!("{} wrote invalid UTF-8: {e}", script.display()))
 }
 
-/// Cases where `mdscrub` is meant to differ from the Python, each one
-/// argued for in `KNOWN.md`.
-pub const DIVERGENT_MDSCRUB: &[&str] = &["breaks"];
-
 /// Every case file for a tool, in a fixed order.
 pub fn cases(tool: &str) -> Vec<PathBuf> {
     let dir = dir().join("cases").join(tool);

parity/tests/mdscrub.rs

@@ -13,37 +13,18 @@ fn every_case_cleans_the_same_as_python() {
 
     for case in cases {
         let name = parity::case_name(&case);
-        if parity::DIVERGENT_MDSCRUB.contains(&name.as_str()) {
-            continue;
-        }
         let ours = mdscrub::scrub(&case_text(&case));
         let theirs = parity::read_golden_text(&format!("mdscrub/{name}.md"));
         assert_eq!(ours, theirs, "{name} differs from the python");
     }
 }
 
-#[test]
-fn the_documented_divergences_are_exactly_what_was_argued_for() {
-    for name in parity::DIVERGENT_MDSCRUB {
-        let case = parity::dir()
-            .join("cases/mdscrub")
-            .join(format!("{name}.md"));
-        let ours = mdscrub::scrub(&case_text(&case));
-        let theirs = parity::read_golden_text(&format!("mdscrub/{name}.md"));
-        assert_ne!(ours, theirs, "{name} no longer diverges — update KNOWN.md");
-        assert_eq!(
-            ours,
-            parity::read_golden_text(&format!("mdscrub/{name}.rust.md")),
-            "{name} diverges differently than recorded"
-        );
-    }
-}
-
-/// The one thing the divergence exists to prevent.
+/// The thing the break-tag rule exists to prevent.
 #[test]
 fn a_line_break_tag_never_welds_two_words_together() {
     let cleaned = mdscrub::scrub("the mill stood at the<br>northern end");
     assert!(!cleaned.contains("thenorthern"), "{cleaned:?}");
+    assert!(cleaned.contains("the northern"), "{cleaned:?}");
 }
 
 /// Catches a golden file drifting away from a Python that has moved on.

parity/tests/unescape.rs

@@ -0,0 +1,57 @@
+//! Character references must decode exactly as Python decodes them.
+//!
+//! The table is two thousand entries of specification data and the rules
+//! around it exist only because browsers had to accept what was already on
+//! the web. Getting one of them wrong leaves markup in the text of a book,
+//! which is how `<sup>107` reached thousands of passages before anyone
+//! noticed.
+
+#[test]
+fn every_reference_decodes_the_same_as_python() {
+    let cases = std::fs::read_to_string(parity::dir().join("cases/unescape/entities.txt"))
+        .expect("a readable case file");
+    let cases: Vec<&str> = cases.split('\n').collect();
+    let golden = parity::read_golden_text("unescape/entities.jsonl");
+    let golden: Vec<String> = golden
+        .lines()
+        .map(|line| serde_json::from_str(line).expect("the oracle writes JSON strings"))
+        .collect();
+
+    assert_eq!(
+        cases.len(),
+        golden.len(),
+        "{} cases against {} golden answers — regenerate",
+        cases.len(),
+        golden.len()
+    );
+
+    let mut wrong = Vec::new();
+    for (case, expected) in cases.iter().zip(&golden) {
+        let got = mdscrub::unescape::unescape(case);
+        if &got != expected {
+            wrong.push(format!("{case:?}: python {expected:?}, rust {got:?}"));
+        }
+    }
+    assert!(
+        wrong.is_empty(),
+        "{} of {} references differ:\n{}",
+        wrong.len(),
+        cases.len(),
+        wrong
+            .iter()
+            .take(20)
+            .cloned()
+            .collect::<Vec<_>>()
+            .join("\n")
+    );
+}
+
+/// Catches a golden file drifting away from a Python that has moved on.
+#[test]
+#[cfg(feature = "python")]
+fn the_golden_file_still_matches_live_python() {
+    let cases = std::fs::read_to_string(parity::dir().join("cases/unescape/entities.txt"))
+        .expect("a readable case file");
+    let live = parity::run_oracle("unescape.py", &cases).expect("the oracle runs");
+    assert_eq!(live, parity::read_golden_text("unescape/entities.jsonl"));
+}