Skip to content

Add processing pipeline test spec with golden outputs#434

Open
eyeseast wants to merge 4 commits into
masterfrom
claude/processing-pipeline-test-spec-24amep
Open

Add processing pipeline test spec with golden outputs#434
eyeseast wants to merge 4 commits into
masterfrom
claude/processing-pipeline-test-spec-24amep

Conversation

@eyeseast

Copy link
Copy Markdown
Contributor

Summary

Adds documentcloud/documents/processing/tests/spec/: an executable version of the processing pipeline output contract researched in MuckRock/research/processing-pipeline-spec. The goal is to be able to change processing code and validate that the pipeline's outputs stay the same (or that a diff shows exactly what changed).

For each test document under documents/, an expected/ directory contains every file the current pipeline produces — processed PDF, concatenated and per-page text, txt.json, position.json, all five page image sizes, the .index cache — plus metadata.json recording the database-facing API callbacks (page_count, page_spec, file_hash, status).

How it works

  • harness.py runs the real serverless functions (info_and_image, ocr) in-process against local storage and Redis. Nothing in the pipeline is mocked: pdfium rendering, Tesseract OCR (the LFS-bundled libtesseract), grafting, and pdfplumber positions all run for real. Pubsub topics dispatch through a FIFO queue, mirroring production's one-Lambda-per-message model.
  • test_spec.py (pytest) reruns the pipeline for every case and compares against the goldens. It runs in CI with the existing Redis service; OCR cases skip automatically when the LFS Tesseract libraries or pinned traineddata aren't present (CI doesn't check out LFS, so CI covers the embedded-text cases).
  • generate.py regenerates the goldens after an intentional behavior change; generate.py --check validates without pytest.
  • compare.py encodes the normalization rules for known non-determinism: updated timestamps, PDF bytes (uuid-named XObjects from the grafter, pikepdf /ID), page_spec segment order (Redis set iteration), and the redacted-PDF file_hash.

Test corpus (8 cases)

Embedded text (1 and 3 pages), scanned/image-only, mixed text+scan, force_ocr, mixed page dimensions, blank page, and a redaction pass. Inputs are built deterministically by make_corpus.py and committed (~1.7 MB total including goldens).

Notable pipeline behaviors the goldens capture: pdfium text ends with a trailing \x00, Tesseract text ends with \f (a blank page yields "\f", not ""), force_ocr produces ocr: "tess4_force", and redaction re-sends file_hash/status but not page_spec.

Not covered yet

Textract OCR, non-PDF conversion (needs the LibreOffice LFS bundle), page modifications (needs storage.async_download in local storage), bulk import, set_page_text, and non-English OCR — all documented in the README.

Test plan

  • All 8 cases pass pytest documentcloud/documents/processing/tests/spec/test_spec.py repeatedly across fresh processes
  • generate.py --check reproduces the committed goldens byte-for-byte (modulo the documented normalizations)

🤖 Generated with Claude Code

https://claude.ai/code/session_01Le7iTGKrY5TmE4jL4GhnWU


Generated by Claude Code

claude added 2 commits July 14, 2026 15:58
Add documentcloud/documents/processing/tests/spec/: an executable version
of the processing pipeline output contract researched in
MuckRock/research/processing-pipeline-spec.

Each test document under documents/ has an input PDF and an expected/
directory containing every file the current pipeline produces for it
(processed PDF, concatenated and per-page text, txt.json, position.json,
all five page image sizes, index cache) plus the database-facing metadata
captured from the API callbacks (page_count, page_spec, file_hash, status).

The harness runs the real serverless functions in-process - pdfium
rendering, Tesseract OCR, grafting, and text position extraction are not
mocked - against local storage and Redis, with pubsub topics dispatched
through a FIFO queue to mirror one-Lambda-per-message production behavior.

The corpus covers embedded text, scanned, mixed, force-OCR, mixed page
sizes, blank page, and redaction cases. test_spec.py validates a fresh
pipeline run against the goldens (OCR cases skip when the LFS Tesseract
libraries or pinned traineddata are unavailable); generate.py regenerates
the goldens after intentional behavior changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Le7iTGKrY5TmE4jL4GhnWU
Pull the Tesseract LFS libraries in the CI test job (only the ocr/tesseract
directory, to avoid fetching the large LibreOffice bundle) so the OCR test
spec cases run in CI instead of skipping. The pinned eng.traineddata is
downloaded automatically on first use. Also fix pylint findings in the spec
package and drop an unused compare_case parameter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Le7iTGKrY5TmE4jL4GhnWU
@eyeseast

eyeseast commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Review: gaps in coverage, measured against the goal of pipeline replaceability

The goal this spec serves: be able to swap out parts of the pipeline — or replace it entirely — and have confidence the new system produces the same final outputs. This review evaluates the PR against that goal, not against internal implementation details (callback ordering, message choreography, batch sizes are explicitly not the contract).

What holds up well

  • The harness genuinely runs the real pipeline, and the CI claims check out (-m "not solr" means the slow-marked spec runs in CI; ENVIRONMENT: local is set workflow-wide; the USE_TIMEOUT default-argument trap in error_handling.py is avoided because env is set before import).
  • The merged database dict in metadata.json is the right abstraction for final outputs: it captures what the API ends up believing about the document, regardless of how many messages or PATCHes it took to get there. A replacement pipeline is correctly not required to reproduce the current one's message sequence.
  • The normalization rules in compare.py match real sources of non-determinism (timestamps, Redis set ordering, uuid-named XObjects, pikepdf IDs).
  • The architecture already has the right seam: harness.py is necessarily coupled to the current implementation, but compare.py + the documents/ corpus are pure input→expected-output pairs with no dependency on the pipeline at all. That's the durable asset a replacement would be validated against.

Gaps, ranked

1. The comparison rules encode "same implementation, unchanged" — not "equivalent output." GIF comparison is byte-exact, so a replacement renderer (or even a pdfium version bump) fails on every image; OCR text is byte-exact, so a different Tesseract build fails on every scanned page. Those rules are right for regression-testing the current pipeline, but for replacement confidence a second, looser mode is needed: perceptual/tolerance image comparison, OCR similarity thresholds rather than equality. The pieces are already there — _compare_images already computes pixel deltas, it just treats any delta as failure. A strictness knob (exact vs. equivalent) would let the same goldens serve both purposes.

2. The goldens enshrine implementation quirks as contract without deciding whether they are contract. The README documents them honestly — pdfium's trailing \x00, Tesseract's trailing \f, blank pages yielding "\f" — but for replacement purposes each needs a verdict: is the NUL terminator something downstream consumers depend on (search indexing, the frontend, add-ons), or an accident a new system should be free to fix? If it's an accident, exact-byte text comparison forces a replacement to faithfully reproduce a bug. Deciding quirk-by-quirk (and normalizing the accidents in compare.py) is what turns these goldens from a snapshot into a spec.

3. Corpus breadth is the biggest coverage gap for replacement confidence. Eight synthetic PyMuPDF documents exercise the happy path of the exact libraries that generated them. A replacement could match all eight and still diverge on real-world documents: rotated pages, non-Latin/RTL text, ligatures and odd encodings, scanned pages with skew and noise, PDFs with forms or broken xref tables, JPEG2000 images, very large page dimensions. A handful of real (redistributable) documents would carry more weight than any harness improvement.

4. No error-path case, and it's not in the README's "not covered" list. "Given a corrupt/truncated/password-protected input, the system reports an error state to the API" is a final-output contract a replacement must honor, and it's currently untested and undocumented. The harness is already positioned for it: collect_metadata captures the error POSTs; the test just asserts they're empty. At minimum this belongs in the README's gap list.

5. The redacted file_hash is ignored entirely rather than shape-checked (test_spec.py:71). Ignoring its value is correct (the rewritten PDF is non-reproducible), but "a file_hash must be present and well-formed after redaction" is a final-output requirement — a replacement that never set it would currently pass.

6. Make the replacement seam explicit in the README. A short section stating the boundary — the corpus and compare_case(expected, actual) are implementation-independent; to validate a replacement, write a runner that stages the input and produces a document directory plus final metadata — would make the intended use real rather than implicit.

Harness robustness (minor)

  • OCR readiness checks presence, not loadability (harness.py:120-126). The LFS files are Linux .so libraries; on a macOS checkout with LFS pulled, ocr_ready() returns True and ctypes.CDLL raises OSError — an error, not the promised skip. Relatedly, the byte-exact GIF goldens are implicitly pinned to CI's pdfium/Pillow builds; the README could say "run in the container or on Linux."
  • CI downloads eng.traineddata from GitHub raw on every run (test_spec.py:39-42) — hash-pinned (good), but uncached, and the failure mode is silent: if the download fails, all six OCR cases skip and CI goes green covering only the two embedded-text cases. Consider caching spec/.cache/ and failing (not skipping) in CI where OCR is expected to work.
  • case["needs_ocr"] raises KeyError if a future case.json omits it — load_case could default it. And on test failure the actual/ output tree is deleted (test_spec.py:77), so you only get the problem strings; keeping the temp dir on failure would make diffing a failed golden much easier.

Acknowledged and reasonable

The README's "not covered" list (Textract, non-PDF conversion, page modifications, import, set_page_text, non-English OCR) matches the uncovered entry points in main.py. Of those, page modifications is the one to prioritize next: it's user-triggered like redaction, shares the dirty-page reprocessing machinery, and the stated blocker (storage.async_download missing from local storage) looks like a small shim rather than a fundamental obstacle.

None of these are blockers. Items 1–3 are direction-setting for the replaceability goal; items 4–5 are the concrete contract holes worth closing soonest.

🤖 Generated with Claude Code

- Compare the full API callback sequence in metadata.json, not just the
  merged database fields: order, methods, URLs, and payloads must match.
  page_spec values are compared decoded, file_hash values are loosened to
  presence + SHA-1 shape for redaction cases (the redacted PDF is rewritten
  non-reproducibly), and storage bucket prefixes are stripped from error
  messages. This makes regressions like redaction resending page_spec or
  dropping its file_hash update detectable.
- Add corrupt-1page: an unparseable PDF must produce an error POST to the
  API and no output files. The runner neutralizes the local sentry stub's
  re-raise so the error path completes as it does in production.
- Add text-4page: four pages exceed TEXT_POSITION_BATCH (3), exercising the
  multi-batch flush path for text position extraction.
- ocr_libraries_available now verifies the libraries actually load (they
  are Linux x86-64 builds), so non-Linux checkouts skip instead of error.
- CI: cache the pinned eng.traineddata and set DC_SPEC_REQUIRE_OCR so an
  unavailable OCR runtime fails the OCR cases instead of skipping them.
- Keep the actual output tree on test failure for easier diffing; default
  needs_ocr/expects_error in load_case; document access levels, remaining
  error paths, and the Linux platform pinning as known gaps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Le7iTGKrY5TmE4jL4GhnWU

Copy link
Copy Markdown
Contributor Author

Addressed in 7353e79 — all seven items:

  1. Callback sequence is now compared (compare.py:_compare_callbacks): order, methods, URLs, and payloads must match. page_spec values are compared decoded; for redaction cases file_hash values are loosened to presence + 40-hex shape instead of being dropped, so redaction resending page_spec or omitting a file_hash now fails the test.
  2. Error-path case added (corrupt-1page): an unparseable PDF must produce exactly one error POST to documents/{id}/errors/ (message compared with the bucket prefix stripped, since bucket names vary by environment) and no output files. The runner neutralizes the local sentry stub's re-raise so the error path completes as it does in production.
  3. text-4page added: four pages exceed TEXT_POSITION_BATCH (3); the generation log confirms the multi-batch flush ([0, 1, 2] then [3]). The README's IMAGE_BATCH claim is corrected — multi-batch image extraction is genuinely not exercised and is now listed as a gap.
  4. Access levels documented as a gap in the README.
  5. OCR readiness now checks loadability, not just file presence — a macOS checkout with LFS pulled skips instead of erroring. The Linux pinning of the GIF goldens is called out in a platform note.
  6. CI caches spec/.cache/ (actions/cache, keyed to the pinned hash) and sets DC_SPEC_REQUIRE_OCR=1, so an unavailable OCR runtime fails the OCR cases in CI instead of silently skipping to green.
  7. needs_ocr/expects_error default in load_case, and on failure the actual output tree is kept and its path included in the assertion message for diffing.

All 10 cases pass locally (and 5 pass + 5 skip cleanly with LFS pointers, matching what a plain checkout sees); pylint 10/10, isort/black clean. Agreed on page modifications as the next corpus addition — the async_download shim for local storage looks small.


Generated by Claude Code

compare_case now takes strictness="exact" (default; regression testing
the current pipeline, byte-exact) or strictness="equivalent" (validating
a modified or replacement pipeline against the same goldens):

- trailing control characters (pdfium's NUL, Tesseract's form feed) are
  normalized away, treating them as implementation accidents rather than
  contract; OCR text is held to a similarity threshold instead of equality
- images require the same dimensions and a small mean pixel delta rather
  than identical bytes
- positions must parse, stay in the 0-1 range, agree on emptiness, and
  carry approximately the same text
- the txt.json ocr field is compared as OCR'd-or-not rather than by engine
  name; file_hash must be well-formed but is not compared by value; the
  callback sequence is not compared, though error reports still must be
  sent

Exposed via generate.py --check --equivalent. test_spec.py and CI stay on
exact mode. The README documents the replacement seam (corpus + compare are
implementation-independent; harness.py is the current pipeline's runner)
and lists real-world corpus breadth and quirk verdicts as open items.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Le7iTGKrY5TmE4jL4GhnWU

Copy link
Copy Markdown
Contributor Author

Follow-up for the replaceability framing, in 5fe986f:

1 (strictness knob)compare_case now takes strictness="exact" (default; what test_spec.py/CI run) or "equivalent", exposed as generate.py --check --equivalent. Equivalent mode: OCR text held to a similarity ratio (0.90) instead of equality; images to same dimensions + mean pixel delta ≤ 4/255 instead of identical bytes; positions must parse, stay in range, agree on emptiness, and carry ~the same text; PDF text layer by similarity; ocr compared as OCR'd-or-not rather than engine name; file_hash must be well-formed but isn't compared by value; the callback sequence isn't compared, but error reports must still be sent. Thresholds are constants at the top of compare.py, documented as a first cut. Sanity-checked that it tolerates a simulated replacement (trailing \f stripped, minor OCR drift, renamed engine, different hash — 4 exact-mode failures, 0 equivalent-mode) while still catching a real text change.

6 (seam) — README now has a "Validating a modified or replacement pipeline" section stating the boundary explicitly: documents/ + compare_case are implementation-independent; harness.py is the current pipeline's runner; a replacement gets its own runner producing the same directory layout + metadata.json.

2 (quirk verdicts) — took a default position rather than deciding for the team: exact mode pins the quirks (NUL, \f) so regressions are visible; equivalent mode normalizes them away, treating them as accidents a replacement may fix. The README flags the open question — if a downstream consumer (search indexing, frontend, add-ons) turns out to depend on one, it should move into the contract and the rule tightened. Happy to go audit the consumers if you want that settled now.

3 (real-world corpus) — agreed this is the biggest gap; added to the README's gap list as the highest-value improvement. Picking small redistributable real documents (rotated, RTL, skewed scans, broken xref, JPEG2000) is a curation call — if you point me at candidates (or say "grab N public-domain docs from production"), I'll add them and generate goldens.

Items 4–5 and the robustness notes were addressed in 7353e79 (error-path case, shape-checked redaction file_hash via the callback comparison, OCR loadability check, traineddata caching + DC_SPEC_REQUIRE_OCR in CI, needs_ocr default, kept temp dir on failure).


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants