Merge pull request #9 from artiz/claude/pdf-2305-improvements

fix(pdf): recover text dropped by ff-ligatures and degenerate font metrics (2305)

Author: artiz

PDF_CONFORMANCE.md

@@ -48,6 +48,27 @@ to ONNX in `tableformer.rs`) on a cv2-exact preprocessed crop (`resample.rs`); t
 structure + matched cell text reproduce docling's padded GitHub tables (2305-pg9
 is cell-for-cell exact).
 
+### Performance / parallelism
+
+Profiling a 14-page document (`FLEISCHWOLF_TIMING=1` prints an env-gated per-stage
+wall-clock breakdown) shows ~80 % of the time is the two ONNX models (layout ~58 %,
+TableFormer ~22 %) and ~16 % the page-image downsample — all per-page work that is
+independent across pages. A multi-page PDF therefore renders on one thread (pdfium
+is not thread-safe) and fans the pages out across a **pool of page-workers**, each
+owning its own model set (`ort`'s `Session::run` is `&mut self`, so sessions can't
+be shared), reassembled in page order. A bounded channel keeps only a handful of
+page bitmaps resident, so the streaming memory profile is preserved; the output is
+byte-identical to the serial path (verified across all PDF snapshots). Single-page /
+image / METS inputs keep the serial path and load no helper models.
+
+The layout model is **memory-bandwidth bound** (even one model at four intra-op
+threads only reaches ~2.1× core utilisation), so the pool defaults to two intra-op
+threads per worker with `workers ≈ cores / 2` (capped at 4): two threads sharing one
+in-cache copy of the weights beats both one fat model and many single-thread workers.
+The speed-up scales with cores and memory bandwidth. Tune per machine with
+`FLEISCHWOLF_PDF_WORKERS` (pool size) and `FLEISCHWOLF_PDF_INTRA` (intra-op threads
+per worker).
+
 ### Text reconstruction: a pure-Rust PDF text parser (default)
 
 The byte-exact ceiling was the **text extractor** — pdfium's *rendered* glyph

README.md

@@ -143,7 +143,11 @@ The ML formats (PDF, images, METS) need pdfium + the ONNX models, so they are
 covered by a separate **deterministic snapshot** harness rather than `cargo test`:
 
 ```bash
-bash scripts/pdf_setup.sh           # one-time: fetch pdfium + the ONNX models
+bash scripts/pdf_setup.sh           # one-time: fetch pdfium + export the ONNX models
+                                    # (layout + TableFormer; needs a torch/docling Python)
+# Updating an existing checkout after a model-format change (e.g. the cached
+# TableFormer decoder): `rm -rf models/tableformer && bash scripts/pdf_setup.sh`,
+# or re-run `python scripts/export_tableformer.py models/tableformer` directly.
 
 export PDFIUM_DYNAMIC_LIB_PATH="$(pwd)/.pdfium/lib"
 export DOCLING_LAYOUT_ONNX="$(pwd)/models/layout_heron.onnx"

crates/fleischwolf-cli/src/main.rs

@@ -19,13 +19,14 @@
 use std::path::Path;
 use std::process::ExitCode;
 
-use fleischwolf::{DocumentConverter, ImageMode, SourceDocument};
+use fleischwolf::{DocumentConverter, ImageMode, InputFormat, Pipeline, SourceDocument};
 
 fn main() -> ExitCode {
     let mut strict = false;
     let mut to = "md".to_string();
     let mut images = "placeholder".to_string();
     let mut fetch_images = false;
+    let mut bench_warm: Option<usize> = None;
     let mut path: Option<String> = None;
     let mut args = std::env::args().skip(1);
     while let Some(arg) = args.next() {
@@ -34,6 +35,17 @@ fn main() -> ExitCode {
             "--fetch-images" => fetch_images = true,
             "--to" => to = args.next().unwrap_or_default(),
             "--images" => images = args.next().unwrap_or_default(),
+            // Hidden benchmarking aid: load the PDF/image pipeline once, then time
+            // N warm conversions (models already loaded), printing the avg seconds
+            // per conversion to stdout. This is the startup-excluded counterpart to
+            // Python docling's in-process "warm" measurement, for a fair head-to-head.
+            "--bench-warm" => {
+                bench_warm = args.next().and_then(|n| n.parse::<usize>().ok());
+                if bench_warm.is_none() {
+                    eprintln!("error: --bench-warm needs a positive run count");
+                    return ExitCode::from(2);
+                }
+            }
             _ => path = Some(arg),
         }
     }
@@ -67,6 +79,24 @@ fn main() -> ExitCode {
         }
     };
 
+    if let Some(runs) = bench_warm {
+        return match bench_warm_conversion(&source, runs) {
+            Ok(avg) => {
+                // Bare seconds on stdout for the benchmark harness; a human line on stderr.
+                println!("{avg:.6}");
+                eprintln!(
+                    "warm conversion: {:.4}s/doc over {runs} runs (startup excluded)",
+                    avg
+                );
+                ExitCode::SUCCESS
+            }
+            Err(e) => {
+                eprintln!("error: {e}");
+                ExitCode::FAILURE
+            }
+        };
+    }
+
     let document = match DocumentConverter::new()
         .strict(strict)
         .fetch_images(fetch_images)
@@ -109,3 +139,35 @@ fn main() -> ExitCode {
     print!("{md}");
     ExitCode::SUCCESS
 }
+
+/// Build the PDF/image pipeline once (loading the ONNX models), then time `runs`
+/// warm conversions and return the average seconds per conversion. The first
+/// conversion is a discarded warm-up that triggers the lazy model loads, so the
+/// timed runs reuse them — the startup-excluded figure comparable to docling's
+/// in-process warm number.
+fn bench_warm_conversion(source: &SourceDocument, runs: usize) -> Result<f64, String> {
+    let mut pipeline = Pipeline::new().map_err(|e| e.to_string())?;
+    let once = |p: &mut Pipeline| -> Result<(), String> {
+        match source.format {
+            InputFormat::Pdf => p
+                .convert(&source.bytes, None, &source.name)
+                .map(|_| ())
+                .map_err(|e| e.to_string()),
+            InputFormat::Image => p
+                .convert_image(&source.bytes, &source.name)
+                .map(|_| ())
+                .map_err(|e| e.to_string()),
+            other => Err(format!(
+                "--bench-warm supports PDF/image only, not {other:?}"
+            )),
+        }
+    };
+    once(&mut pipeline)?; // warm-up: load models, prime caches
+    let mut total = 0.0f64;
+    for _ in 0..runs {
+        let t = std::time::Instant::now();
+        once(&mut pipeline)?;
+        total += t.elapsed().as_secs_f64();
+    }
+    Ok(total / runs as f64)
+}

crates/fleischwolf-pdf/src/assemble.rs

@@ -5,7 +5,7 @@
 //! assigned to its best-containing region, regions are ordered in reading order
 //! (two-column aware), and each becomes a typed node by its layout label.
 
-use fleischwolf_core::{DoclingDocument, Node, PictureImage, Table};
+use fleischwolf_core::{Node, PictureImage, Table};
 
 use crate::layout::Region;
 use crate::pdfium_backend::{PdfPage, TextCell};
@@ -455,20 +455,27 @@ fn region_text(region: &Region, cells: &[TextCell]) -> String {
             let h = (c.b - c.t).abs().max((p.b - p.t).abs()).max(1.0);
             let gap = if rtl { p.l - c.r } else { c.l - p.r };
             // Dehyphenate a wrapped word: a line ending in a hyphen/dash followed
-            // by a lowercase continuation joins without the dash or a space
-            // (`platforms—` + `reflects` → `platformsreflects`). The dash is still
-            // raw here (clean_text normalizes em/en dashes later), so match them all.
+            // by a continuation joins without the dash or a space (`platforms—` +
+            // `reflects` → `platformsreflects`). The dash is still raw here
+            // (clean_text normalizes em/en dashes later), so match them all.
             let ends_dash = matches!(
                 joined.chars().last(),
                 Some('-' | '\u{2010}' | '\u{2013}' | '\u{2014}')
             );
+            let before = joined.chars().nth_back(1); // char before the dash
+            let next = t.chars().next();
             let dehyph = dp
                 && ends_dash
-                && joined
-                    .chars()
-                    .nth_back(1)
-                    .is_some_and(|c| c.is_alphabetic())
-                && t.chars().next().is_some_and(|c| c.is_lowercase());
+                && before.is_some_and(|c| c.is_alphabetic())
+                && next.is_some_and(|n| {
+                    // Ordinary hyphenation (lowercase continuation), or a CamelCase
+                    // compound name wrapped at the hyphen — a lowercase letter before
+                    // the dash continuing with an uppercase one (`PubTab-Net` →
+                    // `PubTabNet`, `Table-Former` → `TableFormer`). Excludes runs like
+                    // `MS-COCO` (uppercase before the dash) and `PubTables-1M` (digit).
+                    n.is_lowercase()
+                        || (n.is_uppercase() && before.is_some_and(|b| b.is_lowercase()))
+                });
             if dehyph {
                 joined.pop();
             } else if dp || !same_band || gap > h * 0.25 {
@@ -656,10 +663,10 @@ pub fn assemble_page(
     page: &PdfPage,
     regions: Vec<Region>,
     table_rows: &[Option<Vec<Vec<String>>>],
-    doc: &mut DoclingDocument,
-) {
+) -> (Vec<Node>, Vec<(String, String)>) {
+    let mut nodes: Vec<Node> = Vec::new();
     // Recover this page's hyperlinks (rendered only in strict Markdown).
-    doc.links.extend(resolve_link_anchors(page));
+    let links = resolve_link_anchors(page);
     // Pair each region with its precomputed TableFormer grid (indexed by original
     // order) and order by reading order together, so they stay aligned.
     let mut items: Vec<(Region, Option<Vec<Vec<String>>>)> = regions
@@ -697,9 +704,9 @@ pub fn assemble_page(
             let caption = caption_for[i]
                 .map(|ci| region_text(&regions[ci], &page.cells))
                 .filter(|t| !t.is_empty());
-            doc.push(Node::Picture {
+            nodes.push(Node::Picture {
                 caption,
-                image: crop_region(page, region),
+                image: crate::timing::timed("crop_region", || crop_region(page, region)),
             });
             continue;
         }
@@ -710,7 +717,7 @@ pub fn assemble_page(
         match region.label {
             // docling renders both the document title and section headers as
             // `##` (it never emits a top-level `#` for PDFs), so match that.
-            "title" | "section_header" => doc.push(Node::Heading {
+            "title" | "section_header" => nodes.push(Node::Heading {
                 level: 2,
                 text: md_escape(&text),
             }),
@@ -723,15 +730,15 @@ pub fn assemble_page(
                     .trim_start()
                     .to_string();
                 if let Some((number, rest)) = parse_ordered_marker(&stripped) {
-                    doc.push(Node::ListItem {
+                    nodes.push(Node::ListItem {
                         ordered: true,
                         number,
                         first_in_list: false,
                         text: md_escape(&rest),
                         level: 0,
                     });
                 } else {
-                    doc.push(Node::ListItem {
+                    nodes.push(Node::ListItem {
                         ordered: false,
                         number: 0,
                         first_in_list: false,
@@ -752,11 +759,11 @@ pub fn assemble_page(
                         vec![vec![text.clone()]]
                     }
                 });
-                doc.push(Node::Table(Table { rows }));
+                nodes.push(Node::Table(Table { rows }));
             }
             // docling does not decode formulas in the standard pipeline; it emits
             // a placeholder comment rather than the (garbled) raw glyph text.
-            "formula" => doc.push(Node::Paragraph {
+            "formula" => nodes.push(Node::Paragraph {
                 text: "<!-- formula-not-decoded -->".into(),
             }),
             // Code blocks: use the space-glyph-only grouping (monospace keeps its
@@ -772,24 +779,25 @@ pub fn assemble_page(
                     .replace(" ;", ";")
                     .replace(" )", ")")
                     .replace(" (", "(");
-                doc.push(Node::Code {
+                nodes.push(Node::Code {
                     language: None,
                     text: code,
                 });
                 // docling emits the `Listing N:` caption after the code block.
                 if let Some(ci) = code_caption_for[i] {
                     let cap = region_text(&regions[ci], &page.cells);
                     if !cap.is_empty() {
-                        doc.push(Node::Paragraph { text: cap });
+                        nodes.push(Node::Paragraph { text: cap });
                     }
                 }
             }
             // text, caption, footnote → paragraph
-            _ => doc.push(Node::Paragraph {
+            _ => nodes.push(Node::Paragraph {
                 text: md_escape(&text),
             }),
         }
     }
+    (nodes, links)
 }
 
 /// Merge paragraph fragments split across a column or page break. docling joins a
@@ -800,13 +808,31 @@ pub fn assemble_page(
 /// past a figure resumes below it (`…The wing type that is` ⟶[figure]⟶ `the most
 /// common…`), and docling emits the whole paragraph before the figure. A heading,
 /// table, or list between them ends the paragraph (no merge).
+/// A paragraph that is really a figure/table caption (`Fig. 1. …`, `Table 2 …`).
+/// Used to skip an unpaired caption when stitching a paragraph that wraps around
+/// a figure.
+fn looks_like_caption(text: &str) -> bool {
+    let head: String = text.trim_start().chars().take(14).collect();
+    (head.starts_with("Fig") || head.starts_with("Table"))
+        && head.contains(|c: char| c.is_ascii_digit())
+}
+
 pub(crate) fn merge_continuations(nodes: &mut Vec<Node>) {
     let mut i = 0;
     while i + 1 < nodes.len() {
         let Node::Paragraph { text: a } = &nodes[i] else {
             i += 1;
             continue;
         };
+        // A figure/table caption is a self-contained unit; body text resuming
+        // after a figure is the continuation case, not the caption itself. Never
+        // stitch *from* a caption — otherwise a caption that ends in a lone glyph
+        // (`Fig. 5. … PubTabNet. μ`) would swallow a following stray figure label
+        // (a standalone `μ`) into `… μ μ`.
+        if looks_like_caption(a) {
+            i += 1;
+            continue;
+        }
         // Open if the fragment ends mid-word (a letter) or with a wrap hyphen/dash
         // — docling joins `vocab-` + `ulary` → `vocab- ulary`.
         let a_open = a.trim_end().chars().next_back().is_some_and(|c| {
@@ -817,9 +843,13 @@ pub(crate) fn merge_continuations(nodes: &mut Vec<Node>) {
             continue;
         }
         // The continuation is the next paragraph, looking past any figures the
-        // text wraps around (but nothing else).
+        // text wraps around — and a figure/table caption that was emitted as its
+        // own paragraph (an above-the-figure caption that didn't pair), since the
+        // body text resumes after the whole figure+caption block.
         let mut j = i + 1;
-        while matches!(nodes.get(j), Some(Node::Picture { .. })) {
+        while matches!(nodes.get(j), Some(Node::Picture { .. }))
+            || matches!(nodes.get(j), Some(Node::Paragraph { text }) if looks_like_caption(text))
+        {
             j += 1;
         }
         let cont = matches!(nodes.get(j), Some(Node::Paragraph { text: b })

crates/fleischwolf-pdf/src/layout.rs

@@ -54,13 +54,20 @@ pub struct LayoutModel {
 impl LayoutModel {
     /// Load the ONNX model from `DOCLING_LAYOUT_ONNX` (or `models/layout_heron.onnx`).
     pub fn load() -> Result<Self, String> {
+        Self::load_with(crate::intra_threads())
+    }
+
+    /// Like [`load`](Self::load) but with an explicit intra-op thread count. A
+    /// parallel page-worker pool loads its helper models on a single thread each
+    /// and gets its speed-up from running pages concurrently instead.
+    pub fn load_with(intra: usize) -> Result<Self, String> {
         let path = std::env::var("DOCLING_LAYOUT_ONNX")
             .unwrap_or_else(|_| "models/layout_heron.onnx".to_string());
         let session = Session::builder()
             .map_err(|e| format!("layout: builder: {e}"))?
             // Let inference use the available cores (ort otherwise defaults low);
             // a large PDF runs this model once per page.
-            .with_intra_threads(crate::intra_threads())
+            .with_intra_threads(intra)
             .map_err(|e| format!("layout: intra_threads: {e}"))?
             .commit_from_file(&path)
             .map_err(|e| format!("layout: load {path}: {e}"))?;