fix(ledger-core): PdfIngestOp actually calls a real command now, real bridge - #200
Merged
Merged
Conversation
… bridge
Previously PdfIngestOp spawned `reqif-opa-mcp ingest --file <path> --output
ndjson` and parsed stdout as NDJSON ReqIfCandidate lines. Neither the
reqif-opa-mcp binary nor an `ingest` subcommand exist in the real CLI
(confirmed: no [project.scripts] entry point in its pyproject.toml) — this
path had never actually run end-to-end, it would always fail to spawn.
It also built TransactionInput { date: candidate.section, amount:
candidate.confidence.to_string(), .. } from a ReqIfCandidate (a
modal-verb-detected requirement sentence) — type-checked, but a
requirement's confidence score is not a dollar amount, and its section
heading is not a date.
Now: spawns the real `uv run python -m reqif_ingest_cli extract <path>
--profile auto` in a configurable reqif_opa_mcp_dir, parses the single
DoclingDocumentGraph JSON blob it prints (per docling_bridge, added in
#199), classifies every node via bank_statement::classify_document, and
bridges only TransactionRow-classified nodes into real TransactionInput
values via node_to_transaction_input — the actual date/description/amount
extracted from the matched text, not placeholder garbage.
PdfIngestOp gained two new required fields: reqif_opa_mcp_dir (path to a
reqif-opa-mcp checkout) and account_id (statements carry no reliable
self-identifying account number the PDF text alone can recover).
Added pdf_ingest_op_real_subprocess_extraction_and_classification, an
#[ignore]'d integration test (requires uv + a real reqif-opa-mcp checkout)
that spawns the real subprocess against the real OWASP ASVS sample PDF —
the exact path neither of the two prior tests exercised (both return
before reaching the subprocess). Ran it here: passes, proving the
previously-broken command now genuinely executes.
201 existing tests still pass, 0 regressions.
…ngestOp fields CI's clippy --all-targets build caught this: PR #200 added account_id and reqif_opa_mcp_dir to PdfIngestOp but missed the separate tests/pdf_ingest_integration.rs integration-test file (only the inline unit tests in ledger_ops.rs were updated). Uses a deliberately nonexistent reqif_opa_mcp_dir so these CI-environment tests still take the pre-existing ExternalProcessFailed fallback path they were written around.
elasticdotventures
added a commit
that referenced
this pull request
Aug 26, 2026
#204) * fix(mcp): real docling_ready probe + DoclingProcessSurface attestation (#60) PdfIngestOp and DoclingDocumentGraph (the actual PDF-ingest path, via a uv-run reqif-opa-mcp sidecar) already existed before this change, landed by #200. This closes the remaining gap from #60: l3dg3rr_get_pipeline_status hardcoded docling_ready to true instead of checking anything. Adds b00t_iface::docling::DoclingProcessSurface, a ProcessSurface attestation checking the sidecar's two real hard preconditions (uv on PATH, reqif-opa-mcp checkout present) — adapted from #60's literal `which::which("docling")` sketch, which predates the uv/reqif-opa-mcp architecture and no longer matches how PdfIngestOp actually works. Also removes integration_tests.rs's stale #[ignore]'d test_ingest_statement_via_pdf_sidecar, which asserted behavior (IngestStatementOp itself doing PDF ingest) that contradicts the PdfIngestOp design actually shipped; PdfIngestOp has its own coverage in ledger_ops.rs, including a real ignored subprocess integration test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(b00t-iface): make docling init() check order CI-independent CI's test-and-build check failed on init_fails_when_checkout_missing: it asserted Err(DoclingError::CheckoutMissing(_)) with a nonexistent checkout dir, but init() checked `uv` on PATH first, and CI runners don't have uv installed — so it returned NotOnPath instead, which the test didn't expect. Passed locally only because uv happens to be installed on this dev machine. Reorders init() to check the checkout path first, so the test (and init()'s behavior generally) no longer depends on whether uv happens to be present on whatever machine is running it. Verified locally both with uv on PATH and with a PATH that excludes it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PdfIngestOppreviously spawnedreqif-opa-mcp ingest --file <path> --output ndjsonand parsed stdout as NDJSONReqIfCandidatelines. Neither thereqif-opa-mcpbinary nor aningestsubcommand exist in the real CLI (no[project.scripts]entry point in itspyproject.toml) — this path had never actually run end-to-end; it would always fail to spawn.It also built
TransactionInput { date: candidate.section, amount: candidate.confidence.to_string(), .. }from aReqIfCandidate(a modal-verb-detected requirement sentence) — type-checked, but a requirement's confidence score is not a dollar amount, and its section heading is not a date.Fix
Now spawns the real
uv run python -m reqif_ingest_cli extract <path> --profile autoin a configurablereqif_opa_mcp_dir, parses the singleDoclingDocumentGraphJSON blob it prints (perdocling_bridge, #199), classifies every node viabank_statement::classify_document, and bridges onlyTransactionRow-classified nodes into realTransactionInputvalues vianode_to_transaction_input.PdfIngestOpgained two new required fields:reqif_opa_mcp_dirandaccount_id(a statement's PDF text carries no reliable self-identifying account number).Test plan
#[ignore]'d integration test spawns the real subprocess against the real OWASP ASVS sample PDF in a realreqif-opa-mcpcheckout — the exact path neither prior test exercised. Ran it here: passes.