mdscrub: residual html, control characters, headings, page markers

parity/Cargo.toml

@@ -15,6 +15,7 @@ python = []
 
 [dependencies]
 chunkmd.workspace = true
+mdscrub.workspace = true
 corpus-core.workspace = true
 serde_json.workspace = true
 textkit.workspace = true

parity/KNOWN.md

@@ -18,4 +18,65 @@ affected and how often it occurs in practice.
 
 ---
 
-None recorded yet.
+## 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
+pagination in as `<!-- Page 1 -->\n\n…`, so that downstream can assume a
+marker exists.
+
+**Rust:** text with no page separators and no form feeds is returned
+unchanged.
+
+**Why this is acceptable:** a cleanup filter has to be safe to run twice, and
+the Python's version is not — a second pass wraps the first pass's marker in
+another one. The assumption it exists to serve belongs to the converter that
+knows it just read a PDF, not to a filter that is handed markdown of unknown
+provenance. The chunker already handles a document with no page markers by
+emitting chunks with no page number, which is the honest answer.
+
+**Measured:** not exercised by any parity case, since the Python function is
+only reachable from the PDF conversion path. Covered by
+`mdscrub::pages::tests::text_with_no_page_boundaries_is_left_exactly_as_it_was`.
+
+## chunkmd, role and footnote patterns — `\s` excludes four control characters
+
+**Python:** `\s` in a `str` pattern matches the Unicode `White_Space`
+property *and* `U+001C`–`U+001F`, the four ASCII information separators.
+
+**Rust:** `regex` and `fancy-regex` match the `White_Space` property only, so
+those four characters are not whitespace to a ported pattern.
+
+**Why this is acceptable:** every one of those codepoints is deleted by
+`mdscrub` before a chunker sees it — they fall inside the control-character
+class that a text column rejects. For the pattern to behave differently, a
+document would have to reach `chunkmd` without having been cleaned, and carry
+an information separator in exactly the position where a pattern expects
+whitespace.
+
+Note that this does **not** apply to the sentence and paragraph splitters in
+`textkit`, which match Python's definition exactly — see
+`textkit::segment::is_space`. Those decide chunk boundaries, so the four
+characters are handled there rather than argued about.
+
+**Measured:** zero differences over the parity corpus; the sample in
+`parity/src/sample.rs` includes `U+001C` deliberately and the token counts
+agree over all ten thousand lines.

parity/cases/mdscrub/anchors.md

@@ -0,0 +1,11 @@
+# <span id="page12"></span>The Quarrel
+
+The mill stood at the northern end of the valley.
+
+<span id="page13"></span>
+
+Two miles downstream the abbey had built its own.
+
+## <a id="page-14"></a>
+
+Text under a heading that was nothing but an anchor.

parity/cases/mdscrub/blanks.md

@@ -0,0 +1,10 @@
+First paragraph.
+
+
+
+
+Second paragraph after four blank lines.
+
+<span class="only-a-tag"></span>
+
+Third paragraph after a line that was nothing but a tag.

parity/cases/mdscrub/breaks.md

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

parity/cases/mdscrub/fenced.md

@@ -0,0 +1,14 @@
+Ordinary <em>prose</em> before the fence.
+
+```html
+<span class="kept">This is an example and must survive.</span>
+<a href="#x">So must this.</a>
+```
+
+Ordinary <b>prose</b> after the fence.
+
+~~~xml
+<note epub:type="footnote">Also kept.</note>
+~~~
+
+The end <i>of it</i>.

parity/cases/mdscrub/headings.md

@@ -0,0 +1,11 @@
+# [I. The Setting](#ch1) {.unnumbered}
+
+## [The Quarrel]{.chapter-title}
+
+### **THE SETTLEMENT**
+
+#### <span class="head">A   Spaced   Heading</span>
+
+#not a heading
+
+Body text follows, and is left alone apart from its tags.

parity/cases/mdscrub/mixed.md

@@ -0,0 +1,12 @@
+# [<span id="page3"></span>III. The Second Estate](#ch3)
+
+The obligation ran <em>both ways</em>, and contemporaries said so plainly
+whenever they were asked.<a href="#n4"><sup>4</sup></a>
+
+<span id="page4"></span>
+
+What the rolls do not say is how often the obligation was tested.
+
+## [Notes]{.section-title}
+
+4. Bloch, *Feudal Society*, vol. 2, p. 17.

parity/cases/mdscrub/tags.md

@@ -0,0 +1,11 @@
+<span class="class_s28E">The</span> second <em>estate</em> held its land in
+exchange for <strong>service</strong>, and the service was
+<a href="#n1"><sup>1</sup></a> military.
+
+<div class="section">
+
+A paragraph inside a div that means nothing once the styling is gone.
+
+</div>
+
+Text with an angle bracket in it: a < b and c > d, which is not a tag.

parity/golden/mdscrub/anchors.md

@@ -0,0 +1,13 @@
+# The Quarrel
+
+<!-- Page 12 -->
+
+The mill stood at the northern end of the valley.
+
+<!-- Page 13 -->
+
+Two miles downstream the abbey had built its own.
+
+<!-- Page 14 -->
+
+Text under a heading that was nothing but an anchor.

parity/golden/mdscrub/blanks.md

@@ -0,0 +1,5 @@
+First paragraph.
+
+Second paragraph after four blank lines.
+
+Third paragraph after a line that was nothing but a tag.

parity/golden/mdscrub/breaks.md

@@ -0,0 +1,5 @@
+The address was set with explicit line breaks:Twelve Mill LaneThe
+Valleyand 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.

parity/golden/mdscrub/breaks.rust.md

@@ -0,0 +1,9 @@
+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/golden/mdscrub/control.md

@@ -0,0 +1,3 @@
+Text with a null and a vertical tab and a shift-out in it.
+
+A second paragraph, clean.

parity/golden/mdscrub/fenced.md

@@ -0,0 +1,14 @@
+Ordinary prose before the fence.
+
+```html
+<span class="kept">This is an example and must survive.</span>
+<a href="#x">So must this.</a>
+```
+
+Ordinary prose after the fence.
+
+~~~xml
+<note epub:type="footnote">Also kept.</note>
+~~~
+
+The end of it.

parity/golden/mdscrub/headings.md

@@ -0,0 +1,11 @@
+# I. The Setting
+
+## The Quarrel
+
+### THE SETTLEMENT
+
+#### A Spaced Heading
+
+#not a heading
+
+Body text follows, and is left alone apart from its tags.

parity/golden/mdscrub/mixed.md

@@ -0,0 +1,14 @@
+# III. The Second Estate
+
+<!-- Page 3 -->
+
+The obligation ran both ways, and contemporaries said so plainly
+whenever they were asked.4
+
+<!-- Page 4 -->
+
+What the rolls do not say is how often the obligation was tested.
+
+## Notes
+
+4. Bloch, *Feudal Society*, vol. 2, p. 17.

parity/golden/mdscrub/tags.md

@@ -0,0 +1,7 @@
+The second estate held its land in
+exchange for service, and the service was
+1 military.
+
+A paragraph inside a div that means nothing once the styling is gone.
+
+Text with an angle bracket in it: a < b and c > d, which is not a tag.

parity/oracle/mdscrub.py

@@ -0,0 +1,26 @@
+"""Clean converted markdown the way the Python pipeline does.
+
+Reads markdown on stdin, writes the cleaned markdown on stdout.
+
+    CATENA_PARITY_PYTHONPATH=/path/to/book-pipeline/src \
+        python3 oracle/mdscrub.py < case.md
+"""
+
+import os
+import sys
+
+for path in os.environ.get("CATENA_PARITY_PYTHONPATH", "").split(os.pathsep):
+    if path:
+        sys.path.insert(0, path)
+
+from book_pipeline.convert import _epub_postprocess  # noqa: E402
+
+
+def main() -> int:
+    markdown = sys.stdin.buffer.read().decode("utf-8")
+    sys.stdout.write(_epub_postprocess(markdown))
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

parity/src/bin/regen.rs

@@ -19,7 +19,11 @@ type Written = Result<Vec<String>, String>;
 type Job = (&'static str, fn() -> Written);
 
 fn main() -> ExitCode {
-    let jobs: [Job; 2] = [("tokens", tokens), ("chunkmd", chunkmd)];
+    let jobs: [Job; 3] = [
+        ("tokens", tokens),
+        ("chunkmd", chunkmd),
+        ("mdscrub", mdscrub),
+    ];
 
     for (name, job) in jobs {
         match job() {
@@ -50,6 +54,31 @@ fn tokens() -> Written {
     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())?;
+
+    let mut written = Vec::new();
+    for case in parity::cases("mdscrub") {
+        let markdown = std::fs::read_to_string(&case).map_err(|e| e.to_string())?;
+        let name = parity::case_name(&case);
+        let cleaned = parity::run_oracle("mdscrub.py", &markdown)?;
+        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)
+}
+
 fn chunkmd() -> Written {
     let dir = parity::golden("chunkmd");
     std::fs::create_dir_all(&dir).map_err(|e| e.to_string())?;

parity/src/lib.rs

@@ -83,6 +83,10 @@ 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

@@ -0,0 +1,59 @@
+//! Cleaned markdown must match the Python byte for byte, except where the
+//! Rust is meant to differ — and there, the difference must be exactly the
+//! one that was argued for.
+
+fn case_text(case: &std::path::Path) -> String {
+    std::fs::read_to_string(case).expect("a readable case")
+}
+
+#[test]
+fn every_case_cleans_the_same_as_python() {
+    let cases = parity::cases("mdscrub");
+    assert!(!cases.is_empty(), "no cases found");
+
+    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.
+#[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:?}");
+}
+
+/// Catches a golden file drifting away from a Python that has moved on.
+#[test]
+#[cfg(feature = "python")]
+fn the_golden_files_still_match_live_python() {
+    for case in parity::cases("mdscrub") {
+        let name = parity::case_name(&case);
+        let live = parity::run_oracle("mdscrub.py", &case_text(&case)).expect("the oracle runs");
+        let golden = parity::read_golden_text(&format!("mdscrub/{name}.md"));
+        assert_eq!(live, golden, "golden file for {name} is stale");
+    }
+}