a missing word, a hidden dependency, and a second pass that changed things

crates/mdscrub/Cargo.toml

@@ -11,6 +11,7 @@ repository.workspace = true
 catena-cli.workspace = true
 clap.workspace = true
 corpus-core.workspace = true
+fancy-regex.workspace = true
 regex.workspace = true
 textkit.workspace = true
 

crates/mdscrub/src/heading.rs

@@ -8,8 +8,9 @@
 
 use std::sync::LazyLock;
 
+use fancy_regex::Regex as Boundaried;
 use regex::Regex;
-use textkit::pattern::SPACE;
+use textkit::pattern::{SPACE, WORD_END, WORD_START};
 
 fn pattern(source: &str) -> Regex {
     Regex::new(source).expect("a valid pattern")
@@ -36,10 +37,11 @@ static HEADING: LazyLock<Regex> = LazyLock::new(|| pattern(&format!(r"^(#{{1,6}}
 
 /// A page anchor: `<span id="…page42">`, `<a id="page-42">`, and the rest of
 /// the shapes an EPUB uses to say where a printed page began.
-pub static PAGE_ANCHOR: LazyLock<Regex> = LazyLock::new(|| {
-    pattern(&format!(
-        r#"(?i)<(?:span|a|div)\b[^>]*\bid{SPACE}*={SPACE}*["'][^"']*?page[-_]?(\d+)[^"']*["'][^>]*>(?:</(?:span|a|div)>)?"#
+pub static PAGE_ANCHOR: LazyLock<Boundaried> = LazyLock::new(|| {
+    Boundaried::new(&format!(
+        r#"(?i)<(?:span|a|div){WORD_END}[^>]*{WORD_START}id{SPACE}*={SPACE}*["'][^"']*?page[-_]?(\d+)[^"']*["'][^>]*>(?:</(?:span|a|div)>)?"#
     ))
+    .expect("a valid pattern")
 });
 
 /// Clean one heading's text.
@@ -71,6 +73,7 @@ pub fn clean_all(markdown: &str) -> String {
         let raw = &found[2];
         let pages: Vec<u32> = PAGE_ANCHOR
             .captures_iter(raw)
+            .filter_map(Result::ok)
             .map(|anchor| textkit::decimal(&anchor[1]))
             .collect();
         let cleaned = clean(&PAGE_ANCHOR.replace_all(raw, ""));
@@ -86,10 +89,15 @@ pub fn clean_all(markdown: &str) -> String {
 }
 
 /// Turn the page anchors left in the body into page markers.
+///
+/// The number is carried through exactly as the document wrote it, padding
+/// and all: `page007` becomes page 007. A heading's anchor is read as a
+/// number instead, because a heading's page is a fact about the document
+/// rather than a piece of its text.
 pub fn anchors_to_markers(markdown: &str) -> String {
     PAGE_ANCHOR
-        .replace_all(markdown, |found: &regex::Captures| {
-            format!("\n\n<!-- Page {} -->\n\n", textkit::decimal(&found[1]))
+        .replace_all(markdown, |found: &fancy_regex::Captures| {
+            format!("\n\n<!-- Page {} -->\n\n", &found[1])
         })
         .into_owned()
 }
@@ -141,6 +149,12 @@ mod tests {
         assert!(cleaned.contains("<!-- Page 19 -->"));
     }
 
+    #[test]
+    fn a_padded_page_number_in_the_body_keeps_its_padding() {
+        let cleaned = anchors_to_markers("text <span id=\"page007\"></span> more");
+        assert!(cleaned.contains("<!-- Page 007 -->"), "{cleaned}");
+    }
+
     #[test]
     fn a_line_that_is_not_a_heading_is_untouched() {
         assert_eq!(

crates/mdscrub/src/lib.rs

@@ -20,8 +20,7 @@ pub mod unescape;
 ///
 /// Safe to run twice: the second run changes nothing.
 pub fn scrub(markdown: &str) -> String {
-    let text = pages::normalize(markdown);
-    let text = heading::clean_all(&text);
+    let text = heading::clean_all(markdown);
     let text = heading::anchors_to_markers(&text);
     let text = html::strip_tags(&text);
     let text = html::scrub_control(&text);
@@ -62,10 +61,27 @@ mod tests {
     }
 
     #[test]
-    fn form_feeds_become_pages_rather_than_disappearing() {
-        // The control-character pass would delete a form feed, so page
-        // normalisation has to happen first or the boundary is lost.
-        let cleaned = scrub("one\u{c}two");
-        assert!(cleaned.contains("<!-- Page 2 -->"), "{cleaned:?}");
+    fn page_normalisation_is_not_part_of_cleaning() {
+        // A form feed in ordinary markdown is a control character, not a page
+        // boundary. It is one only to a program that knows it just read a PDF.
+        assert_eq!(scrub("one\u{c}two"), "onetwo\n");
+    }
+
+    #[test]
+    fn pagination_first_then_cleaning_is_still_idempotent() {
+        let raw = "one\u{c}<span>two</span>";
+        let once = scrub(&pages::normalize(raw));
+        assert!(once.contains("<!-- Page 2 -->"), "{once:?}");
+        assert_eq!(scrub(&pages::normalize(&once)), once);
+    }
+
+    #[test]
+    fn nothing_manufactures_a_page_boundary_on_a_second_run() {
+        // Stripping a tag can leave a separator flush against a newline. If
+        // pagination ran after that, a second pass would find a boundary the
+        // first pass did not.
+        let raw = "{2}------------------------------<span></span>\nafter\n";
+        let once = scrub(raw);
+        assert_eq!(scrub(&once), once, "{once:?}");
     }
 }

crates/mdscrub/src/main.rs

@@ -10,6 +10,10 @@ use clap::Parser;
 ///
 /// Reads markdown on stdin, or from a path, and writes it to stdout. Safe to
 /// run twice.
+///
+/// Page boundaries are left alone unless `--pages` asks for them, because
+/// only the program that just read a PDF knows that a form feed in the text
+/// is a page break rather than a stray control character.
 #[derive(Parser)]
 #[command(
     version,
@@ -22,6 +26,12 @@ Example:
 struct Args {
     /// Markdown file to read. Reads stdin when absent.
     path: Option<PathBuf>,
+
+    /// First turn page separators and form feeds into page markers. For text
+    /// straight out of a PDF or DjVu extractor, which marks its page
+    /// boundaries that way.
+    #[arg(long)]
+    pages: bool,
 }
 
 fn main() -> ExitCode {
@@ -38,5 +48,9 @@ fn main() -> ExitCode {
         }
     };
 
+    let markdown = match args.pages {
+        true => mdscrub::pages::normalize(&markdown),
+        false => markdown,
+    };
     catena_cli::text("mdscrub", &mdscrub::scrub(&markdown))
 }

crates/pdftriage/src/lib.rs

@@ -44,7 +44,7 @@ const SPARSE_CHARS_PER_PAGE: f64 = 200.0;
 const COMMON_WORDS: &[&str] = &[
     "a", "all", "an", "and", "are", "as", "at", "be", "been", "but", "by", "for", "from", "had",
     "have", "his", "in", "is", "it", "more", "not", "of", "on", "one", "or", "that", "the",
-    "there", "they", "this", "to", "was", "were", "which", "with", "you",
+    "there", "they", "this", "to", "was", "we", "were", "which", "with", "you",
 ];
 const HEALTHY_WORD_RATIO: f64 = 0.10;
 
@@ -133,16 +133,18 @@ pub fn triage(sample: &Sample<'_>, limits: &Limits) -> Triage {
         sample.middle_pages
     ));
 
-    if reflect::undo(sample.middle).is_some() {
-        notes.push("reflecting the character codes recovers ordinary text".into());
-        return Triage {
-            layer: TextLayer::ReflectedFont,
-            needs_ocr: false,
-            notes,
-        };
-    }
-
+    // Damage can only be judged where there is enough text to judge it, and
+    // that guard comes first — including before the recovery attempt, which
+    // on a short sample would otherwise answer for the whole document.
     if middle.len() >= limits.min_chars_to_judge {
+        if reflect::undo(sample.middle).is_some() {
+            notes.push("reflecting the character codes recovers ordinary text".into());
+            return Triage {
+                layer: TextLayer::ReflectedFont,
+                needs_ocr: false,
+                notes,
+            };
+        }
         let letters = score::letter_share(&middle);
         let latin1 = score::latin1_share(&middle);
         notes.push(format!("letters {letters:.2}, latin-1 {latin1:.2}"));
@@ -248,8 +250,29 @@ mod tests {
     }
 
     #[test]
-    fn the_common_word_list_is_sorted_so_it_can_be_searched() {
+    fn the_common_word_list_is_sorted_and_whole() {
         assert!(COMMON_WORDS.windows(2).all(|pair| pair[0] < pair[1]));
+        assert_eq!(COMMON_WORDS.len(), 37, "a word has gone missing");
+    }
+
+    #[test]
+    fn a_short_sample_is_judged_on_density_rather_than_damage() {
+        // Too little text to call it damaged, which is what the density gate
+        // is for. Answering from the recovery attempt instead would route the
+        // file away from the OCR it needs.
+        let stored: String = "The mill stood at the northern end. "
+            .repeat(4)
+            .chars()
+            .map(|c| {
+                let reflected = 0x120 - c as u32;
+                char::from_u32(reflected)
+                    .filter(|_| (0xa1..=0xff).contains(&reflected))
+                    .unwrap_or(c)
+            })
+            .collect();
+        let found = triage(&sample(&prose(), &stored, Some(400)), &Limits::default());
+        assert_eq!(found.layer, TextLayer::Sparse, "{:#?}", found.notes);
+        assert!(found.needs_ocr);
     }
 
     #[test]

crates/pdftriage/src/main.rs

@@ -43,7 +43,8 @@ struct Args {
     #[arg(long)]
     json: bool,
 
-    /// Say nothing; answer with the exit code alone.
+    /// Write no verdict; answer with the exit code alone. Diagnostics still go
+    /// to stderr, because a silent failure is worse than a quiet one.
     #[arg(long, short)]
     quiet: bool,
 }
@@ -62,11 +63,17 @@ fn main() -> ExitCode {
         }
     };
 
-    if which("pdftotext").is_none() {
-        eprintln!("pdftriage: pdftotext is not on PATH (it ships with poppler)");
-        return ExitCode::from(exit::UNAVAILABLE);
+    for program in ["pdftotext", "pdfinfo"] {
+        if which(program).is_none() {
+            eprintln!("pdftriage: {program} is not on PATH (it ships with poppler)");
+            return ExitCode::from(exit::UNAVAILABLE);
+        }
     }
 
+    // Both of them, because without the page count the sample window falls
+    // back to the front of the book and the density gate cannot run at all —
+    // and a file would then be judged differently on two machines that both
+    // have everything the tool said it needed.
     let pages = page_count(&args.path);
     let (first, last) = pdftriage::middle_window(pages, SPAN);
 
@@ -179,11 +186,41 @@ fn page_count(path: &Path) -> Option<u32> {
         .and_then(|count| count.trim().parse().ok())
 }
 
+/// Find a program on `PATH`, the way a shell would.
+///
+/// Checking that a file exists is not enough: a non-executable file of the
+/// right name shadows the real program and turns a clear "not installed" into
+/// an obscure failure several steps later. On Windows the name on `PATH` also
+/// carries an extension, which is what `PATHEXT` lists.
 fn which(program: &str) -> Option<PathBuf> {
     let path = std::env::var_os("PATH")?;
+    let extensions: Vec<String> = match std::env::var("PATHEXT") {
+        Ok(list) => std::iter::once(String::new())
+            .chain(list.split(';').map(str::to_lowercase))
+            .collect(),
+        Err(_) => vec![String::new()],
+    };
+
     std::env::split_paths(&path)
-        .map(|dir| dir.join(program))
-        .find(|candidate| candidate.is_file())
+        .flat_map(|dir| {
+            extensions
+                .iter()
+                .map(move |extension| dir.join(format!("{program}{extension}")))
+        })
+        .find(|candidate| is_program(candidate))
+}
+
+#[cfg(unix)]
+fn is_program(path: &std::path::Path) -> bool {
+    use std::os::unix::fs::PermissionsExt as _;
+
+    std::fs::metadata(path)
+        .is_ok_and(|meta| meta.is_file() && meta.permissions().mode() & 0o111 != 0)
+}
+
+#[cfg(not(unix))]
+fn is_program(path: &std::path::Path) -> bool {
+    path.is_file()
 }
 
 #[cfg(test)]
@@ -192,7 +229,25 @@ mod tests {
 
     #[test]
     fn a_program_that_exists_is_found_and_one_that_does_not_is_not() {
-        assert!(which("sh").is_some() || which("sh.exe").is_some());
+        assert!(which("sh").is_some(), "every system has a shell");
         assert!(which("definitely-not-a-program-93a1").is_none());
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn a_file_that_is_not_executable_is_not_a_program() {
+        use std::os::unix::fs::PermissionsExt as _;
+
+        let dir = std::env::temp_dir().join("pdftriage-which");
+        std::fs::create_dir_all(&dir).expect("a writable directory");
+        let path = dir.join("not-a-program-8813");
+        std::fs::write(&path, b"#!/bin/sh\n").expect("a writable path");
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644))
+            .expect("permissions to set");
+
+        assert!(!is_program(&path));
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755))
+            .expect("permissions to set");
+        assert!(is_program(&path));
+    }
 }