diff --git a/Cargo.lock b/Cargo.lock index 91d796d..eb7595c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -626,6 +626,7 @@ dependencies = [ "catena-cli", "clap", "corpus-core", + "fancy-regex 0.16.2", "regex", "textkit", ] diff --git a/crates/mdscrub/Cargo.toml b/crates/mdscrub/Cargo.toml index 1cc0e55..394ccce 100644 --- a/crates/mdscrub/Cargo.toml +++ b/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 diff --git a/crates/mdscrub/src/heading.rs b/crates/mdscrub/src/heading.rs index 484fc8c..895c1a8 100644 --- a/crates/mdscrub/src/heading.rs +++ b/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 = LazyLock::new(|| pattern(&format!(r"^(#{{1,6}} /// A page anchor: ``, ``, and the rest of /// the shapes an EPUB uses to say where a printed page began. -pub static PAGE_ANCHOR: LazyLock = LazyLock::new(|| { - pattern(&format!( - r#"(?i)<(?:span|a|div)\b[^>]*\bid{SPACE}*={SPACE}*["'][^"']*?page[-_]?(\d+)[^"']*["'][^>]*>(?:)?"# +pub static PAGE_ANCHOR: LazyLock = LazyLock::new(|| { + Boundaried::new(&format!( + r#"(?i)<(?:span|a|div){WORD_END}[^>]*{WORD_START}id{SPACE}*={SPACE}*["'][^"']*?page[-_]?(\d+)[^"']*["'][^>]*>(?:)?"# )) + .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 = 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: ®ex::Captures| { - format!("\n\n\n\n", textkit::decimal(&found[1])) + .replace_all(markdown, |found: &fancy_regex::Captures| { + format!("\n\n\n\n", &found[1]) }) .into_owned() } @@ -141,6 +149,12 @@ mod tests { assert!(cleaned.contains("")); } + #[test] + fn a_padded_page_number_in_the_body_keeps_its_padding() { + let cleaned = anchors_to_markers("text more"); + assert!(cleaned.contains(""), "{cleaned}"); + } + #[test] fn a_line_that_is_not_a_heading_is_untouched() { assert_eq!( diff --git a/crates/mdscrub/src/lib.rs b/crates/mdscrub/src/lib.rs index 41122ad..f97fa67 100644 --- a/crates/mdscrub/src/lib.rs +++ b/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(""), "{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}two"; + let once = scrub(&pages::normalize(raw)); + assert!(once.contains(""), "{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}------------------------------\nafter\n"; + let once = scrub(raw); + assert_eq!(scrub(&once), once, "{once:?}"); } } diff --git a/crates/mdscrub/src/main.rs b/crates/mdscrub/src/main.rs index e85e545..9e11c0d 100644 --- a/crates/mdscrub/src/main.rs +++ b/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, + + /// 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)) } diff --git a/crates/pdftriage/src/lib.rs b/crates/pdftriage/src/lib.rs index 3affa14..0fa311c 100644 --- a/crates/pdftriage/src/lib.rs +++ b/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] diff --git a/crates/pdftriage/src/main.rs b/crates/pdftriage/src/main.rs index fc53b17..81b8efb 100644 --- a/crates/pdftriage/src/main.rs +++ b/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 { .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 { let path = std::env::var_os("PATH")?; + let extensions: Vec = 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)); + } } diff --git a/parity/KNOWN.md b/parity/KNOWN.md index 15a6027..175d7af 100644 --- a/parity/KNOWN.md +++ b/parity/KNOWN.md @@ -38,25 +38,60 @@ there is no input where it changes a cue the original read. recovered. Across the other eight cases, including deliberately malformed timestamps, the two agree cue for cue. +## pdftriage — a newer Unicode table knows letters an older one does not + +**Python:** `str.isalpha` is true for the letter categories of the Unicode +version the interpreter was built against. + +**Rust:** the same test against a table built from a later Unicode version, +which knows 4,924 code points the older one does not. + +**Why this is acceptable:** the newer answer is the correct one, and the +difference is entirely one-directional — every code point the two disagree +about is a letter that Rust knows and Python does not, never the reverse. It +matters because the letter share is what calls a text layer dead: a book +typeset in Cyrillic Extended-C is judged garbled by the older table, which +sends a perfectly readable text layer for an hour of OCR that can only make it +worse. Erring toward not re-OCRing a book whose text is fine is the right +direction for this gate to err in. + +Both tables will keep moving, which is the other reason this entry exists: the +golden files pin one pair of Unicode versions, and a change to either shows up +as a parity failure rather than as a silently different verdict. + +**Measured:** `parity/cases/pdftriage/samples.ndjson`, case +`cyrillic-extended-c`: letter share 1.00 here against 0.00 there, which is the +difference between healthy and garbled. Brute-forced over the whole code +space: 4,924 disagreements, none of them in the other direction. + ## 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 `\n\n…`, so that downstream can assume a marker exists. -**Rust:** text with no page separators and no form feeds is returned +**Rust:** page boundaries are not touched at all unless `--pages` asks for +them, and even then text with no 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. +provenance: a form feed in a code listing is a control character, and a dash +rule in a quoted document is not a page break. The chunker already handles a +document with no page markers by emitting chunks with no page number, which is +the honest answer. + +Separating the two passes is also what makes the cleanup idempotent. Removing +a tag can leave a dash rule flush against a newline, and a pagination pass +running afterwards would find a page boundary on the second run that was not +there on the first. **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`. +`mdscrub::pages::tests::text_with_no_page_boundaries_is_left_exactly_as_it_was` +and `mdscrub::tests::nothing_manufactures_a_page_boundary_on_a_second_run`. ## Every ported pattern — the two engines disagree about `\d` and `(?i)` @@ -97,6 +132,10 @@ make every chunk record wider to carry a value nothing can produce. The saturation is deliberate rather than a wrap, so the wrong answer is at least obviously wrong. +Note that this applies only where a page number becomes a *number*. Where one +is copied into text — a page anchor in the body of a document — it is carried +through exactly as it was written, padding and all. + **Measured:** no document in any corpus this was run against has a page marker above five digits. diff --git a/parity/cases/pdftriage/samples.ndjson b/parity/cases/pdftriage/samples.ndjson index 7df81ac..27531d0 100644 --- a/parity/cases/pdftriage/samples.ndjson +++ b/parity/cases/pdftriage/samples.ndjson @@ -22,3 +22,5 @@ {"name": "nbsp-prose", "text": "The second estate held its land in exchange for service, and the service was military. That much is agreed; what follows is not. The court rolls do not say how often the obligation was tested, because the clerk who recorded it had no vocabulary for the difference between a rebellion and a bad harvest. This is the central difficulty of the sources, and everything that follows depends on reading around that limitation rather than through it. The second estate held its land in exchange for service, and the service was military. That much is agreed; what follows is not. The court rolls do not say how often the obligation was tested, because the clerk who recorded it had no vocabulary for the difference between a rebellion and a bad harvest. This is the central difficulty of the sources, and everything that follows depends on reading around that limitation rather than through it. "} {"name": "control-chars", "text": "text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c text\u0001with\u000bcontrol\u001fchars\u001c "} {"name": "ligatures", "text": "The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. The first flight of the figure was confirmed. "} +{"name": "function-words", "text": "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 we were which with you 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 we were which with you 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 we were which with you 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 we were which with you "} +{"name": "cyrillic-extended-c", "text": "ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ ᲉᲉᲉᲉᲉ "} diff --git a/parity/golden/pdftriage/samples.ndjson b/parity/golden/pdftriage/samples.ndjson index eb23d48..5cddc7d 100644 --- a/parity/golden/pdftriage/samples.ndjson +++ b/parity/golden/pdftriage/samples.ndjson @@ -22,3 +22,5 @@ {"ascii": 1.0, "body": 736, "decoded": null, "english": 0.42105263157894735, "latin1": 0.0, "letters": 0.9782608695652174, "name": "nbsp-prose", "tidy": 1.0, "vowelled": 1.0} {"ascii": 0.9523809523809523, "body": 630, "decoded": null, "english": 0.0, "latin1": 0.0, "letters": 0.9523809523809523, "name": "control-chars", "tidy": 1.0, "vowelled": 1.0} {"ascii": 0.8823529411764706, "body": 680, "decoded": null, "english": 0.5, "latin1": 0.0, "letters": 0.9705882352941176, "name": "ligatures", "tidy": 1.0, "vowelled": 0.875} +{"ascii": 1.0, "body": 436, "decoded": null, "english": 1.0, "latin1": 0.0, "letters": 1.0, "name": "function-words", "tidy": 1.0, "vowelled": 1.0} +{"ascii": 0.0, "body": 500, "decoded": null, "english": 0.0, "latin1": 0.0, "letters": 0.0, "name": "cyrillic-extended-c", "tidy": null, "vowelled": null} diff --git a/parity/tests/pdftriage.rs b/parity/tests/pdftriage.rs index 5c7cbcd..ab69fe9 100644 --- a/parity/tests/pdftriage.rs +++ b/parity/tests/pdftriage.rs @@ -10,6 +10,12 @@ use serde_json::Value; +/// Measurements that are meant to differ, each argued for in `KNOWN.md`. +/// +/// The two implementations compile against different Unicode versions, and +/// the newer one knows letterforms the older one does not. +const DIVERGENT: &[(&str, &str)] = &[("cyrillic-extended-c", "letters")]; + /// Both sides compute a count divided by a count, so agreement is exact or /// the counts differ. fn same(ours: f64, theirs: &Value) -> bool { @@ -46,6 +52,9 @@ fn every_measurement_matches_python() { let body = pdftriage::score::body(text); let mut note = |field: &str, ours: String, theirs: &Value| { + if DIVERGENT.contains(&(name, field)) { + return; + } wrong.push(format!("{name}.{field}: python {theirs}, rust {ours}")); }; @@ -100,6 +109,29 @@ fn every_measurement_matches_python() { } } + // A divergence that has quietly gone away is a note in KNOWN.md that is + // no longer true, which is worth knowing about. + for (name, field) in DIVERGENT { + let case = cases + .iter() + .find(|case| case["name"].as_str() == Some(name)) + .unwrap_or_else(|| panic!("{name} is recorded as divergent but is not a case")); + let theirs = golden + .iter() + .find(|record| record["name"].as_str() == Some(name)) + .expect("a golden answer"); + let body = pdftriage::score::body(case["text"].as_str().expect("text")); + let ours = match *field { + "letters" => pdftriage::score::letter_share(&body), + other => panic!("nothing measures {other}"), + }; + assert_ne!( + Some(ours), + theirs[field].as_f64(), + "{name}.{field} no longer diverges — update KNOWN.md" + ); + } + assert!( wrong.is_empty(), "{} measurements differ:\n{}",