feat(pdf-extract): scaffold single-source stranske-pdf-extract library (#2711) - #2716
Conversation
Single-source PDF text-extraction for the fleet, replacing four diverging implementations (Counter_Risk, Pension-Data, Inv-Man-Intake, Manager-Database). Ships the generalized result + page-level-provenance contract (generalizes Pension-Data's evidence model + Inv-Man-Intake's bbox provenance + Protocols), the lifted fallback-ladder orchestration primitive, a greenfield reliability layer (arithmetic/business-rule checks + cross-check + calibration/routing, absent fleet-wide), a real Docling provider behind the Protocol (optional dep), a pure-python baseline, and a golden-set eval harness. 27 deterministic tests pass; named conformance gate proven via deliberate-break. Distribution: pip-installable subdirectory package (not sync-manifest copy-sync) — see packages/stranske_pdf_extract/docs/DESIGN.md. Tracking: #2711. Migrations: #2712 #2713 #2714 #2715. Cross-links IMI #713. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds a new Changesstranske-pdf-extract library
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Automated Status SummaryHead SHA: 46b1b3d
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopePDF text-EXTRACTION is independently reimplemented in four fleet repos with four divergent
Missing/duplicated behavior: items 1–4 of the shape (text-extraction ladder, OCR fallback, orchestration, Tasks
Acceptance criteria
|
|
Runner dispatch state for autofix on PR #2716. Do not edit. |
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stranske_pdf_extract/docs/DESIGN.md`:
- Around line 41-52: The package-tree fence in the DESIGN.md snippet is missing
a language tag, which keeps markdownlint failing. Update the fenced block for
the `stranske_pdf_extract/` tree to use a plain-text language label such as
`text` or `plaintext`, keeping the content unchanged. Locate the fenced package
tree near the `contract.py` / `provider.py` / `orchestration.py` listing and
apply the language tag there.
- Around line 108-111: Update the calibration + confidence routing section in
DESIGN.md to match the shipped reliability.py API: describe only
expected_calibration_error() and route_by_confidence(), remove the mention of a
ConfidenceCalibrator seam, and align the routing thresholds with the current
defaults of accept_at=0.95 and reject_below=0.50 instead of ~85%. Keep the
wording focused on the available functions and current router behavior so the
documentation matches the implemented symbols.
In `@packages/stranske_pdf_extract/README.md`:
- Around line 51-52: The README wording for Docling is inaccurate: the provider
test in test_docling_provider.py::test_docling_provider_conforms_to_protocol
does not “pass with or without” the extra, and the extraction path in the
Docling provider raises DoclingUnavailableError when [docling] is missing.
Reword the Docling section to say the test is skipped when Docling is already
installed and that the provider surfaces DoclingUnavailableError if the extra is
absent, so readers understand it does not silently skip.
In `@packages/stranske_pdf_extract/src/stranske_pdf_extract/contract.py`:
- Line 20: The contract module has an unused import of field alongside
dataclass, which causes lint failure. Remove the field import from the import
statement in the contract.py module and keep only the symbols that are actually
used, such as dataclass.
- Around line 287-295: The strict_evidence check in the ExtractedField
validation only verifies that f.evidence exists, so mismatched EvidenceRef
values can still pass. Update the validation path around the current
strict_evidence/f.evidence logic to also inspect the EvidenceRef contents,
adding a dedicated validator for EvidenceRef and enforcing that source_doc_id
matches the current document plus basic page-number sanity before allowing
high-impact fields. Reuse the existing validation pattern in
validate_provider_output() and keep the new checks close to the current
ExtractedField / HIGH_IMPACT_PREFIXES enforcement.
In `@packages/stranske_pdf_extract/src/stranske_pdf_extract/eval/harness.py`:
- Around line 27-36: The numeric normalization in normalize_value() is using
float(), which can merge distinct exact values into the same canonical form.
Update the numeric parsing in normalize_value() to preserve exact decimal
precision using a decimal-based representation instead of float(), while keeping
the existing handling for negatives and cleaned currency/percent strings. Make
sure the returned canonical string is stable and exact so score_against_golden()
and reliability.cross_check() do not treat unequal values as equal.
In `@packages/stranske_pdf_extract/src/stranske_pdf_extract/orchestration.py`:
- Around line 48-70: The ladder in orchestration.py is only treating
stage.parse() exceptions as recoverable, but an is_complete(parsed) exception
still aborts the flow. Update the failure handling around the parse result in
the parse loop so that the is_complete check is also wrapped in the same
stage-failure path, recording a ParserAttempt with the stage_name and
parser_name and then continuing to later parsers. Use the existing
ParserAttempt, stage.parse(), and is_complete(parsed) flow as the place to catch
and classify this error.
In `@packages/stranske_pdf_extract/src/stranske_pdf_extract/provider.py`:
- Around line 1-91: Run Black on the provider.py module to normalize formatting
so CI passes; update the spacing and line wrapping in the affected definitions
such as register_provider, build_provider, and the module docstring/exports to
match Black’s output without changing behavior.
In
`@packages/stranske_pdf_extract/src/stranske_pdf_extract/providers/docling_provider.py`:
- Around line 45-48: The Docling OCR toggle is exposed on
DoclingProvider.__init__, but _extract_real() ignores self._do_ocr and always
creates DocumentConverter() with default behavior. Update
DoclingProvider._extract_real() to pass the stored OCR setting into the Docling
converter so callers can control OCR, or remove the unused do_ocr parameter
entirely if it should not be supported. Use the DoclingProvider and
_extract_real symbols to keep the change localized.
In
`@packages/stranske_pdf_extract/src/stranske_pdf_extract/providers/text_baseline.py`:
- Around line 35-40: The fallback chain in the text extraction flow stops on
non-empty page lists even when they contain only blank strings, so OCR is
skipped for image-only PDFs. Update the page selection logic in the baseline
extractor method that builds pages to treat “all blank” results from
_with_pdfplumber() and _with_pypdf() as unusable, then continue to _with_ocr()
before falling back to _raw_decode(). Keep the existing helper order, but add a
blank-content check on the returned pages so OCR runs whenever the earlier
parsers produce only empty text.
In `@packages/stranske_pdf_extract/src/stranske_pdf_extract/reliability.py`:
- Around line 161-174: Validate inputs in expected_calibration_error before
creating bins: reject n_bins values less than 1 and ensure each confidence in
the pairs passed to the function is within the expected [0.0, 1.0] range instead
of silently bucketing invalid values. Update the input handling around the
expected_calibration_error loop so bad arguments fail fast with a clear
exception, and keep the binning logic unchanged for valid confidences.
In `@packages/stranske_pdf_extract/tests/test_contract.py`:
- Around line 5-23: The test module is still failing Ruff/Black due to import
ordering and helper formatting in test_contract.py. Normalize the import block
around validate_extracted_document_result, validate_provider_output, and the
contract dataclasses to satisfy I001, and simplify the _field helper’s dict
initialization to the preferred literal style to address C408. Also remove the
unused mismatched assignment in the affected test case so F841 is cleared.
In `@packages/stranske_pdf_extract/tests/test_reliability.py`:
- Around line 3-17: The import block in test_reliability.py needs to be
reformatted to satisfy Ruff and Black before merge. Update the imports around
check_date_in_period, check_foots, check_sign, check_weights_sum_to_one,
cross_check, expected_calibration_error, route_by_confidence, and the
pytest/date imports so the file matches the formatter output and passes
lint/format checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 40a74eb5-bed4-485e-bc9e-db7df905cace
📒 Files selected for processing (18)
packages/stranske_pdf_extract/README.mdpackages/stranske_pdf_extract/docs/DESIGN.mdpackages/stranske_pdf_extract/pyproject.tomlpackages/stranske_pdf_extract/src/stranske_pdf_extract/__init__.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/contract.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/eval/__init__.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/eval/harness.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/orchestration.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/provider.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/providers/__init__.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/providers/docling_provider.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/providers/text_baseline.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/reliability.pypackages/stranske_pdf_extract/tests/test_contract.pypackages/stranske_pdf_extract/tests/test_docling_provider.pypackages/stranske_pdf_extract/tests/test_eval.pypackages/stranske_pdf_extract/tests/test_orchestration.pypackages/stranske_pdf_extract/tests/test_reliability.py
|
Autofix updated these files:
|
|
Runner dispatch state for codex on PR #2716. Do not edit. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stranske_pdf_extract/src/stranske_pdf_extract/reliability.py`:
- Around line 105-112: The sign-check logic in the reliability rule evaluator
currently falls through to None for unknown expected modes, which silently
disables validation. Update the function that handles the expected sign check in
reliability.py to explicitly reject any unsupported expected value by raising
ValueError before the non_negative/non_positive comparisons. Keep the existing
RuleViolation behavior for the supported modes and use the same rule/expected
branch so the fix is localized and easy to locate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fc2d74b7-2b3d-4bb1-9608-1e8e6245d7fd
📒 Files selected for processing (8)
packages/stranske_pdf_extract/src/stranske_pdf_extract/contract.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/eval/harness.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/provider.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/providers/docling_provider.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/providers/text_baseline.pypackages/stranske_pdf_extract/src/stranske_pdf_extract/reliability.pypackages/stranske_pdf_extract/tests/test_contract.pypackages/stranske_pdf_extract/tests/test_reliability.py
The autofix bot handled format + the 7 auto-fixable findings; this clears the 3 remaining hidden fixes (F841 dead var, C408 dict()->literal, C416) so `ruff check` is green. No behavior change; 27 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Closer review-fix pass pushed commit d4fa7d2. Addressed the unresolved CodeRabbit threads for:
Local validation:
All addressed review threads were resolved via GraphQL. Fresh GitHub checks and CodeRabbit are running on d4fa7d2; no automation sleep/poll was used. |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
DisagreementNo major disagreements detected. Unique Insights
🔍 LangSmith Traces |
Summary
Scaffolds
stranske-pdf-extract— the single-source PDF text-extraction library for thestranske/*fleet — replacing four independent, diverging implementations (Counter_Risk, Pension-Data, Inv-Man-Intake,
Manager-Database) with one installable package. Implements deliverable 5 (scaffold) of the initiative;
design, distribution decision, and migration plan are in
docs/DESIGN.md.Related to #2711; resolves the build half without closing the source issue.
What's here
contract.pySourceLocation/Protocols — not a third invented contract)provider.pyExtractionProvider/MultiModalExtractionProviderProtocols, injectable OCR seam, name registryorchestration.pyrun_fallback_chainladder primitive (lifted from Pension-Data, already generic)reliability.pyproviders/docling_provider.py[docling]extra) — IMI #713 should consume thisproviders/text_baseline.pyeval/harness.pyDistribution decision
Installable pip package, not Workflows
sync-manifestcopy-sync (seedocs/DESIGN.md§2): optional nativedeps need extras; only 4/13 consumers need it; consumers must pin and migrate independently without breaking
tests. Homed as a subdirectory package; not added to
sync-manifest.yml.Tests
27 deterministic tests, no network, no heavy deps required:
Named conformance gate
tests/test_docling_provider.py::test_docling_provider_conforms_to_protocolis provenfalsifiable via the deliberate-break pattern (commenting out
DoclingProvider.name→ test FAILS → revert →passes), demonstrated during scaffolding.
Follow-ups (tracked, not in this PR)
pdf-extract-v0.1.0.Migrate Counter_Risk OCR ladder onto stranske-pdf-extract (Phase 3) #2714 (Counter_Risk) → Migrate Manager-Database utils/extract.py onto stranske-pdf-extract (Phase 4 — lowest risk) #2715 (Manager-Database). Each keeps its domain parser + existing tests as the gate.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
stranske-pdf-extract, a pip-installable PDF text-extraction library with installable extras for baseline parsing, Docling-backed extraction, OCR, and AWS fallback.Bug Fixes
Tests
Closes #2711
Automated Status Summary
Scope
PDF text-EXTRACTION is independently reimplemented in four fleet repos with four divergent
result contracts and three OCR strategies (verified by reading source, 2026-06-28, and confirmed by a
fresh clone-grep across all branches — exactly four, none missed):
src/counter_risk/parsers/daily_holdings_pdf.py:81(_extract_textladder pdfplumber→pypdf→OCR).src/pension_data/parser/pdf_pipeline.py:455(parse_pdf_to_funded_input) +src/pension_data/extract/orchestration/fallback.py:53(run_fallback_chain) +db/models/provenance.py.src/inv_man_intake/extraction/providers/base.py:204(ExtractionProviderProtocol;:218MultiModalExtractionProvider) — currentpdf_primary.pyextractor is fixture-grade, not real.utils/extract.py:20(_extract_pdf, pdfplumber→str).Missing/duplicated behavior: items 1–4 of the shape (text-extraction ladder, OCR fallback, orchestration,
result/provenance contract) are rebuilt per repo; a reliability layer (arithmetic/business-rule validation +
cross-check + calibrated confidence) is absent in all four. This is latent fragility + duplicated effort,
not a current break. Grounding:
Code/Audits/2026-06-28-fleet-pdf-extraction-survey.mdand…-methodology.md.A validated scaffold (this design) already exists at
packages/stranske_pdf_extract/with 27 passingdeterministic tests; this issue tracks landing it on
mainand finishing the optional-dep paths.Tasks
packages/stranske_pdf_extract/onmain(the validated scaffold):pyproject.toml(extrasbaseline,docling,ocr,textract,schema,eval),src/stranske_pdf_extract/{contract,provider,orchestration,reliability}.py,providers/{docling_provider,text_baseline}.py,eval/harness.py,docs/DESIGN.md,README.md, andtests/.tests/into Workflows CI (a non-default job, e.g. extend.github/workflows/selftest-ci.ymlor add a
packages-pdf-extractjob) runningPYTHONPATH=packages/stranske_pdf_extract/src python -m pytest packages/stranske_pdf_extract/tests.pdf-extract-v0.1.0and document the install URLgit+https://github.com/stranske/Workflows@pdf-extract-v0.1.0#subdirectory=packages/stranske_pdf_extractinREADME.md.providers/docling_provider.py:_extract_real) behind the[docling]extra andadd an opt-in test that runs only when
docling_available()is true (skips cleanly otherwise).Acceptance criteria
packages/stranske_pdf_extract/tests/test_docling_provider.py::test_docling_provider_conforms_to_protocolpasses in CI (asserts
isinstance(DoclingProvider(), MultiModalExtractionProvider)), AND the full package suitecollects a non-zero count and passes (confirmed-green locally: 27 passed).
name = "docling"attribute insrc/stranske_pdf_extract/providers/docling_provider.py(classDoclingProvider). With this change,test_docling_provider.py::test_docling_provider_conforms_to_protocolmust FAIL withassert isinstance(...) == False. Revert and confirm it passes. (Demonstrated locally during scaffolding.)pip install "git+…#subdirectory=packages/stranske_pdf_extract"importsstranske_pdf_extractwith thecore (no extras) and
importofprovidersdoes not requiredocling(onlyextract_modalitiesdoes).Head SHA: d4fa7d2
Latest Runs: ✅ success — Gate
Required: gate: ✅ success