chore(bench): refresh review

.github/workflows/ci.yml

@@ -25,6 +25,9 @@ jobs:
       - name: Run tests
         run: cargo test --verbose
 
+      - name: Test benchmark harness
+        run: python3 -m unittest discover -s scripts/tests
+
   fmt:
     name: Format
     runs-on: ubuntu-latest

README.md

@@ -27,16 +27,18 @@ Evaluated on the [opendataloader-bench](https://github.com/opendataloader-projec
 
 | Engine | Overall | Reading Order (NID) | Tables (TEDS) | Headings (MHS) | Speed (200 docs) |
 |---|---|---|---|---|---|
-| pdf-inspector | 0.83 | 0.89 | 0.66 | 0.74 | 4s |
-| opendataloader | 0.84 | 0.91 | 0.49 | 0.74 | 11s |
+| pdf-inspector | **0.875** | **0.915** | **0.814** | 0.788 | 3.3s |
+| opendataloader | 0.831 | 0.902 | 0.489 | 0.739 | 3.0s |
 | pymupdf4llm | 0.73 | 0.89 | 0.40 | 0.41 | 18s |
-| markitdown | 0.58 | 0.88 | 0.00 | 0.00 | 8s |
+| markitdown | 0.59 | 0.84 | 0.27 | 0.00 | 23s |
 
-For context, engines that use OCR/ML (docling, marker, mineru) score 0.83-0.88 overall but take 2-180 minutes on the same corpus — pdf-inspector reaches the low end of that range without any OCR, in 4 seconds.
+For context, engines that use OCR/ML (docling, marker, mineru) score 0.83-0.88 overall but take 2-180 minutes on the same corpus — pdf-inspector reaches the top of that range without any OCR, in 3.3 seconds.
 
-**Where we do well:** Speed (fastest of all engines), the best table detection of any engine shown, and heading detection now on par with opendataloader. Overall lands within 0.01 of opendataloader at roughly 2.5× the speed.
+**Where we do well:** The best overall, reading-order, and table scores among the direct extraction engines shown.
 
-**Where we lag:** Reading order still trails opendataloader slightly, and table structure trails OCR-based engines that can see visual layout.
+**Where we lag:** Some direct engines remain slightly faster, and OCR-based engines can recover text that has no usable PDF text layer.
+
+Use the [paired benchmark harness](docs/benchmarking.md) to compare two local builds against the exact same corpus and evaluator revision.
 
 ## Quick start
 

docs/benchmarking.md

@@ -0,0 +1,29 @@
+# Benchmarking against OpenDataLoader
+
+The paired harness runs two `pdf2md` binaries through the same local
+OpenDataLoader corpus, evaluates both outputs, and reports aggregate and
+per-document deltas. This avoids comparing results produced from different
+corpus revisions or evaluator versions.
+
+Build a candidate and provide a released or worktree build as the baseline:
+
+```bash
+cargo build --release
+python3 scripts/bench_opendataloader.py \
+  --bench-dir ../opendataloader-bench \
+  --baseline ../pdf-inspector-main/target/release/pdf2md \
+  --candidate target/release/pdf2md \
+  --max-document-regression 0.02 \
+  --json-output /tmp/pdf-inspector-benchmark.json
+```
+
+Pass `--reference-evaluation path/to/evaluation.json` to report the candidate
+delta against another evaluation, and add `--require-reference-lead` to make a
+negative reference delta fail the run. By default, the candidate must not
+regress the baseline overall score or introduce missing predictions. Use
+`--min-overall-delta` to require a specific aggregate gain.
+
+The OpenDataLoader repository is external and keeps its normal
+`prediction/pdf-inspector` output. Paired evaluation copies each run into a
+temporary directory before evaluating it, so the baseline and candidate cannot
+overwrite one another.

docs/python.md

@@ -18,8 +18,8 @@ Built by [Firecrawl](https://firecrawl.dev) to handle text-based PDFs locally in
 
 | Engine | Overall | Reading order | Tables (TEDS) | Headings | Speed |
 |---|---|---|---|---|---|
-| **pdf-inspector** | 0.83 | 0.88 | **0.66** | 0.74 | **4s** |
-| opendataloader | 0.84 | 0.91 | 0.49 | 0.74 | 11s |
+| **pdf-inspector** | **0.875** | **0.915** | **0.814** | 0.788 | 3.3s |
+| opendataloader | 0.831 | 0.902 | 0.489 | 0.739 | 3.0s |
 | pymupdf4llm | 0.73 | 0.89 | 0.40 | 0.41 | 18s |
 
 OCR/ML engines (docling, marker, mineru) score 0.83–0.88 overall but take 2–180 minutes on the same corpus. Full numbers in the [repo README](https://github.com/firecrawl/pdf-inspector#benchmark).

docs/rust-api.md

@@ -18,8 +18,8 @@ Built by [Firecrawl](https://firecrawl.dev) to handle text-based PDFs locally in
 
 | Engine | Overall | Reading order | Tables (TEDS) | Headings | Speed |
 |---|---|---|---|---|---|
-| **pdf-inspector** | 0.83 | 0.88 | **0.66** | 0.74 | **4s** |
-| opendataloader | 0.84 | 0.91 | 0.49 | 0.74 | 11s |
+| **pdf-inspector** | **0.875** | **0.915** | **0.814** | 0.788 | 3.3s |
+| opendataloader | 0.831 | 0.902 | 0.489 | 0.739 | 3.0s |
 | pymupdf4llm | 0.73 | 0.89 | 0.40 | 0.41 | 18s |
 
 OCR/ML engines (docling, marker, mineru) score 0.83–0.88 overall but take 2–180 minutes on the same corpus. Full numbers in the [repo README](https://github.com/firecrawl/pdf-inspector#benchmark).

scripts/bench_opendataloader.py

@@ -0,0 +1,351 @@
+#!/usr/bin/env python3
+"""Run a paired pdf-inspector OpenDataLoader benchmark and report deltas."""
+
+from __future__ import annotations
+
+import argparse
+import json
+import math
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+from typing import Any
+
+
+SCORE_KEYS = (
+    "overall_mean",
+    "nid_mean",
+    "nid_s_mean",
+    "teds_mean",
+    "teds_s_mean",
+    "mhs_mean",
+    "mhs_s_mean",
+)
+
+
+def _non_negative_int(value: str) -> int:
+    parsed = int(value)
+    if parsed < 0:
+        raise argparse.ArgumentTypeError("must be non-negative")
+    return parsed
+
+
+def _non_negative_float(value: str) -> float:
+    parsed = float(value)
+    if not math.isfinite(parsed) or parsed < 0.0:
+        raise argparse.ArgumentTypeError("must be finite and non-negative")
+    return parsed
+
+
+def _finite_float(value: str) -> float:
+    parsed = float(value)
+    if not math.isfinite(parsed):
+        raise argparse.ArgumentTypeError("must be finite")
+    return parsed
+
+
+def _scores(evaluation: dict[str, Any]) -> dict[str, float]:
+    score = evaluation.get("metrics", {}).get("score", {})
+    return {key: float(score[key]) for key in SCORE_KEYS if score.get(key) is not None}
+
+
+def _documents(evaluation: dict[str, Any]) -> dict[str, float]:
+    documents: dict[str, float] = {}
+    for document in evaluation.get("documents", []):
+        overall = document.get("scores", {}).get("overall")
+        if overall is not None:
+            documents[str(document["document_id"])] = float(overall)
+    return documents
+
+
+def compare_evaluations(
+    baseline: dict[str, Any],
+    candidate: dict[str, Any],
+    reference: dict[str, Any] | None = None,
+    *,
+    top: int = 10,
+) -> dict[str, Any]:
+    """Build aggregate and per-document deltas from evaluator JSON payloads."""
+    baseline_scores = _scores(baseline)
+    candidate_scores = _scores(candidate)
+    metric_deltas = {
+        key: candidate_scores[key] - baseline_scores[key]
+        for key in SCORE_KEYS
+        if key in baseline_scores and key in candidate_scores
+    }
+
+    baseline_documents = _documents(baseline)
+    candidate_documents = _documents(candidate)
+    shared = sorted(baseline_documents.keys() & candidate_documents.keys())
+    document_deltas = [
+        {
+            "document_id": document_id,
+            "baseline": baseline_documents[document_id],
+            "candidate": candidate_documents[document_id],
+            "delta": candidate_documents[document_id] - baseline_documents[document_id],
+        }
+        for document_id in shared
+    ]
+    epsilon = 1e-12
+    improvements = sorted(document_deltas, key=lambda item: item["delta"], reverse=True)
+    regressions = sorted(document_deltas, key=lambda item: item["delta"])
+
+    result: dict[str, Any] = {
+        "baseline": baseline_scores,
+        "candidate": candidate_scores,
+        "deltas": metric_deltas,
+        "missing_predictions": {
+            "baseline": int(baseline.get("metrics", {}).get("missing_predictions", 0)),
+            "candidate": int(candidate.get("metrics", {}).get("missing_predictions", 0)),
+        },
+        "documents": {
+            "shared": len(shared),
+            "improved": sum(item["delta"] > epsilon for item in document_deltas),
+            "regressed": sum(item["delta"] < -epsilon for item in document_deltas),
+            "unchanged": sum(abs(item["delta"]) <= epsilon for item in document_deltas),
+            "largest_improvements": [
+                item for item in improvements if item["delta"] > epsilon
+            ][:top],
+            "largest_regressions": [
+                item for item in regressions if item["delta"] < -epsilon
+            ][:top],
+            "worst_regression": next(
+                (item for item in regressions if item["delta"] < -epsilon), None
+            ),
+        },
+    }
+    if reference is not None:
+        reference_scores = _scores(reference)
+        result["reference"] = reference_scores
+        result["candidate_vs_reference"] = {
+            key: candidate_scores[key] - reference_scores[key]
+            for key in SCORE_KEYS
+            if key in candidate_scores and key in reference_scores
+        }
+    return result
+
+
+def evaluate_gates(
+    comparison: dict[str, Any],
+    *,
+    min_overall_delta: float,
+    max_document_regression: float | None,
+    max_missing: int,
+    require_reference_lead: bool,
+) -> list[str]:
+    """Return human-readable gate failures; an empty list means pass."""
+    failures: list[str] = []
+    overall_delta = comparison["deltas"].get("overall_mean")
+    if overall_delta is None or overall_delta < min_overall_delta:
+        failures.append(
+            f"overall delta {overall_delta!r} is below {min_overall_delta:+.6f}"
+        )
+    candidate_missing = comparison["missing_predictions"]["candidate"]
+    if candidate_missing > max_missing:
+        failures.append(
+            f"candidate has {candidate_missing} missing predictions (maximum {max_missing})"
+        )
+    if max_document_regression is not None:
+        regression = comparison["documents"].get("worst_regression")
+        if regression is not None and regression["delta"] < -max_document_regression:
+            failures.append(
+                "largest document regression "
+                f"{regression['document_id']}={regression['delta']:+.6f} "
+                f"exceeds {-max_document_regression:+.6f}"
+            )
+    if require_reference_lead:
+        reference_delta = comparison.get("candidate_vs_reference", {}).get("overall_mean")
+        if reference_delta is None:
+            failures.append("reference overall score is unavailable")
+        elif reference_delta < 0.0:
+            failures.append(
+                f"candidate trails reference overall by {reference_delta!r}"
+            )
+    return failures
+
+
+def _run(command: list[str], *, cwd: Path, env: dict[str, str] | None = None) -> None:
+    print("+", " ".join(command), flush=True)
+    subprocess.run(command, cwd=cwd, env=env, check=True)
+
+
+def _run_engine(
+    *,
+    bench_dir: Path,
+    python: Path,
+    binary: Path,
+    label: str,
+    scratch_root: Path,
+) -> dict[str, Any]:
+    env = os.environ.copy()
+    env["PDF_INSPECTOR_BINARY"] = str(binary)
+    source = bench_dir / "prediction" / "pdf-inspector"
+    if source.exists():
+        if source.is_dir():
+            shutil.rmtree(source)
+        else:
+            source.unlink()
+    _run(
+        [
+            str(python),
+            "src/pdf_parser.py",
+            "--engine",
+            "pdf-inspector",
+            "--log-level",
+            "WARNING",
+        ],
+        cwd=bench_dir,
+        env=env,
+    )
+
+    if not source.is_dir():
+        raise RuntimeError(f"parser did not produce predictions: {source}")
+    destination = scratch_root / label
+    shutil.copytree(source, destination)
+    _run(
+        [
+            str(python),
+            "src/evaluator.py",
+            "--prediction-root",
+            str(scratch_root),
+            "--engine",
+            label,
+            "--log-level",
+            "WARNING",
+        ],
+        cwd=bench_dir,
+    )
+    with (destination / "evaluation.json").open(encoding="utf-8") as handle:
+        return json.load(handle)
+
+
+def _print_report(comparison: dict[str, Any]) -> None:
+    print("\nMetric                 baseline   candidate       delta")
+    print("--------------------  ----------  ----------  ----------")
+    for key in SCORE_KEYS:
+        if key not in comparison["deltas"]:
+            continue
+        print(
+            f"{key:<20}  {comparison['baseline'][key]:>10.6f}  "
+            f"{comparison['candidate'][key]:>10.6f}  "
+            f"{comparison['deltas'][key]:>+10.6f}"
+        )
+    if "reference" in comparison:
+        delta = comparison["candidate_vs_reference"].get("overall_mean")
+        reference = comparison["reference"].get("overall_mean")
+        reference_display = f"{reference:.6f}" if reference is not None else "n/a"
+        delta_display = f"{delta:+.6f}" if delta is not None else "n/a"
+        print(f"\nReference overall: {reference_display}; candidate delta: {delta_display}")
+
+    documents = comparison["documents"]
+    print(
+        "\nDocuments: "
+        f"{documents['improved']} improved, {documents['regressed']} regressed, "
+        f"{documents['unchanged']} unchanged ({documents['shared']} shared)"
+    )
+    for heading, key in (
+        ("Largest improvements", "largest_improvements"),
+        ("Largest regressions", "largest_regressions"),
+    ):
+        print(f"\n{heading}:")
+        rows = documents[key]
+        if not rows:
+            print("  none")
+        for row in rows:
+            print(
+                f"  {row['document_id']}: {row['delta']:+.6f} "
+                f"({row['baseline']:.6f} -> {row['candidate']:.6f})"
+            )
+
+
+def _arguments(argv: list[str] | None = None) -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--bench-dir", type=Path, required=True)
+    parser.add_argument("--baseline", type=Path, required=True)
+    parser.add_argument("--candidate", type=Path, required=True)
+    parser.add_argument("--python", type=Path)
+    parser.add_argument("--reference-evaluation", type=Path)
+    parser.add_argument("--json-output", type=Path)
+    parser.add_argument("--top", type=_non_negative_int, default=10)
+    parser.add_argument("--min-overall-delta", type=_finite_float, default=0.0)
+    parser.add_argument("--max-document-regression", type=_non_negative_float)
+    parser.add_argument("--max-missing", type=_non_negative_int, default=0)
+    parser.add_argument("--require-reference-lead", action="store_true")
+    return parser.parse_args(argv)
+
+
+def main(argv: list[str] | None = None) -> int:
+    args = _arguments(argv)
+    bench_dir = args.bench_dir.resolve()
+    baseline = args.baseline.resolve()
+    candidate = args.candidate.resolve()
+    # Keep the virtualenv launcher path intact. Resolving its symlink would
+    # invoke the underlying system interpreter without the benchmark's site
+    # packages.
+    python = (args.python or bench_dir / ".venv" / "bin" / "python").absolute()
+    for path, description in (
+        (bench_dir / "src" / "pdf_parser.py", "OpenDataLoader parser"),
+        (bench_dir / "src" / "evaluator.py", "OpenDataLoader evaluator"),
+        (baseline, "baseline binary"),
+        (candidate, "candidate binary"),
+        (python, "Python interpreter"),
+    ):
+        if not path.exists():
+            raise SystemExit(f"{description} not found: {path}")
+
+    with tempfile.TemporaryDirectory(prefix="pdf-inspector-opendataloader-") as temporary:
+        scratch_root = Path(temporary)
+        baseline_evaluation = _run_engine(
+            bench_dir=bench_dir,
+            python=python,
+            binary=baseline,
+            label="baseline",
+            scratch_root=scratch_root,
+        )
+        candidate_evaluation = _run_engine(
+            bench_dir=bench_dir,
+            python=python,
+            binary=candidate,
+            label="candidate",
+            scratch_root=scratch_root,
+        )
+
+        reference = None
+        if args.reference_evaluation is not None:
+            with args.reference_evaluation.resolve().open(encoding="utf-8") as handle:
+                reference = json.load(handle)
+
+        comparison = compare_evaluations(
+            baseline_evaluation,
+            candidate_evaluation,
+            reference,
+            top=args.top,
+        )
+
+    _print_report(comparison)
+    if args.json_output is not None:
+        args.json_output.resolve().write_text(
+            json.dumps(comparison, indent=2) + "\n", encoding="utf-8"
+        )
+
+    failures = evaluate_gates(
+        comparison,
+        min_overall_delta=args.min_overall_delta,
+        max_document_regression=args.max_document_regression,
+        max_missing=args.max_missing,
+        require_reference_lead=args.require_reference_lead,
+    )
+    if failures:
+        print("\nBenchmark gate failed:", file=sys.stderr)
+        for failure in failures:
+            print(f"  - {failure}", file=sys.stderr)
+        return 1
+    print("\nBenchmark gate passed.")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

scripts/tests/test_bench_opendataloader.py

@@ -0,0 +1,203 @@
+import io
+import json
+import sys
+import tempfile
+import unittest
+from contextlib import redirect_stderr, redirect_stdout
+from pathlib import Path
+from unittest.mock import patch
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+
+from bench_opendataloader import (
+    _arguments,
+    _print_report,
+    _run_engine,
+    compare_evaluations,
+    evaluate_gates,
+)
+
+
+def evaluation(overall, documents, *, missing=0):
+    return {
+        "metrics": {
+            "score": {
+                "overall_mean": overall,
+                "nid_mean": overall + 0.01,
+            },
+            "missing_predictions": missing,
+        },
+        "documents": [
+            {
+                "document_id": document_id,
+                "scores": {"overall": score},
+            }
+            for document_id, score in documents.items()
+        ],
+    }
+
+
+class ComparisonTests(unittest.TestCase):
+    def test_reports_metric_and_document_deltas(self):
+        baseline = evaluation(0.80, {"a": 0.8, "b": 0.6, "c": 0.7})
+        candidate = evaluation(0.82, {"a": 0.9, "b": 0.5, "c": 0.7})
+
+        result = compare_evaluations(baseline, candidate, top=1)
+
+        self.assertAlmostEqual(result["deltas"]["overall_mean"], 0.02)
+        self.assertEqual(result["documents"]["improved"], 1)
+        self.assertEqual(result["documents"]["regressed"], 1)
+        self.assertEqual(result["documents"]["unchanged"], 1)
+        self.assertEqual(
+            result["documents"]["largest_improvements"][0]["document_id"], "a"
+        )
+        self.assertEqual(
+            result["documents"]["largest_regressions"][0]["document_id"], "b"
+        )
+
+    def test_reference_delta_is_reported(self):
+        baseline = evaluation(0.80, {})
+        candidate = evaluation(0.82, {})
+        reference = evaluation(0.81, {})
+
+        result = compare_evaluations(baseline, candidate, reference)
+
+        self.assertAlmostEqual(
+            result["candidate_vs_reference"]["overall_mean"], 0.01
+        )
+
+    def test_gates_cover_aggregate_document_missing_and_reference(self):
+        comparison = compare_evaluations(
+            evaluation(0.80, {"a": 0.8}),
+            evaluation(0.79, {"a": 0.7}, missing=1),
+            evaluation(0.81, {}),
+        )
+
+        failures = evaluate_gates(
+            comparison,
+            min_overall_delta=0.0,
+            max_document_regression=0.05,
+            max_missing=0,
+            require_reference_lead=True,
+        )
+
+        self.assertEqual(len(failures), 4)
+
+    def test_regression_gate_is_independent_of_report_limit(self):
+        comparison = compare_evaluations(
+            evaluation(0.80, {"a": 0.8}),
+            evaluation(0.80, {"a": 0.7}),
+            top=0,
+        )
+
+        failures = evaluate_gates(
+            comparison,
+            min_overall_delta=0.0,
+            max_document_regression=0.05,
+            max_missing=0,
+            require_reference_lead=False,
+        )
+
+        self.assertEqual(len(failures), 1)
+        self.assertIn("largest document regression", failures[0])
+
+    def test_report_handles_reference_without_overall_score(self):
+        result = compare_evaluations(
+            evaluation(0.80, {}),
+            evaluation(0.82, {}),
+            {"metrics": {"score": {"nid_mean": 0.81}}},
+        )
+
+        output = io.StringIO()
+        with redirect_stdout(output):
+            _print_report(result)
+
+        self.assertIn("Reference overall: n/a; candidate delta: n/a", output.getvalue())
+
+    def test_reference_gate_reports_missing_score_as_unavailable(self):
+        comparison = compare_evaluations(
+            evaluation(0.80, {}),
+            evaluation(0.82, {}),
+        )
+
+        failures = evaluate_gates(
+            comparison,
+            min_overall_delta=0.0,
+            max_document_regression=None,
+            max_missing=0,
+            require_reference_lead=True,
+        )
+
+        self.assertEqual(failures, ["reference overall score is unavailable"])
+
+    def test_arguments_reject_negative_counts_and_allow_zero_top(self):
+        required = [
+            "--bench-dir",
+            ".",
+            "--baseline",
+            "baseline",
+            "--candidate",
+            "candidate",
+        ]
+        self.assertEqual(_arguments(required + ["--top", "0"]).top, 0)
+        for option in ("--top", "--max-document-regression", "--max-missing"):
+            with self.subTest(option=option), redirect_stderr(io.StringIO()):
+                with self.assertRaises(SystemExit):
+                    _arguments(required + [option, "-1"])
+
+    def test_arguments_reject_nonfinite_float_thresholds(self):
+        required = [
+            "--bench-dir",
+            ".",
+            "--baseline",
+            "baseline",
+            "--candidate",
+            "candidate",
+        ]
+        for option in ("--min-overall-delta", "--max-document-regression"):
+            for value in ("nan", "inf", "-inf"):
+                with self.subTest(option=option, value=value), redirect_stderr(
+                    io.StringIO()
+                ):
+                    with self.assertRaises(SystemExit):
+                        _arguments(required + [option, value])
+
+    def test_run_engine_clears_stale_predictions_before_parser(self):
+        with tempfile.TemporaryDirectory() as temporary:
+            root = Path(temporary)
+            bench_dir = root / "bench"
+            source = bench_dir / "prediction" / "pdf-inspector"
+            source.mkdir(parents=True)
+            (source / "stale.md").write_text("stale", encoding="utf-8")
+            scratch = root / "scratch"
+            scratch.mkdir()
+
+            def fake_run(command, *, cwd, env=None):
+                if any(part.endswith("pdf_parser.py") for part in command):
+                    self.assertFalse(source.exists())
+                    (source / "markdown").mkdir(parents=True)
+                    (source / "markdown" / "new.md").write_text(
+                        "new", encoding="utf-8"
+                    )
+                else:
+                    destination = scratch / "candidate"
+                    (destination / "evaluation.json").write_text(
+                        json.dumps(evaluation(0.82, {})), encoding="utf-8"
+                    )
+
+            with patch("bench_opendataloader._run", side_effect=fake_run):
+                result = _run_engine(
+                    bench_dir=bench_dir,
+                    python=Path("python"),
+                    binary=Path("pdf2md"),
+                    label="candidate",
+                    scratch_root=scratch,
+                )
+
+            self.assertEqual(result["metrics"]["score"]["overall_mean"], 0.82)
+            self.assertFalse((source / "stale.md").exists())
+            self.assertFalse((scratch / "candidate" / "stale.md").exists())
+
+
+if __name__ == "__main__":
+    unittest.main()