feat(ledger-core): typed Docling bridge + deterministic bank-statement classifier - #199
Merged
Merged
Conversation
…t classifier
Adds two modules:
- docling_bridge: typed mirror of reqif-opa-mcp's DocumentGraph JSON
schema (models.py), matching the real shape returned by both
`reqif_ingest_cli extract` and the new ledgrrr-docling NATS Micro
Service. Supersedes rule_registry::DocumentChunk, which declared the
same intent but was never deserialized anywhere and didn't match the
real named-field SourceAnchor shape.
- bank_statement: procedural (regex, never LLM) NodeCategory classifier
over DoclingNode via ufo_types::Satisfies<CategoryConstraint>, plus
node_to_transaction_input bridging TransactionRow-classified nodes into
the existing TransactionInput type.
Fixes a real bug in PdfIngestOp::execute's TransactionInput
construction: it built {date: candidate.section, amount:
candidate.confidence.to_string(), ..} from a ReqIfCandidate — a
modal-verb-detected requirement sentence has no real transaction date
or dollar amount, so this type-checked but was semantically garbage
(a confidence score is not a dollar amount). The new bridge only
accepts nodes procedurally classified as TransactionRow and extracts
real date/description/amount via the same regex used to classify them.
NodeCategory::sarif_subtypes() mirrors reqif-opa-mcp's SARIF
properties.subtypes tagging convention (reqif_mcp/sarif_producer.py),
so classification here is round-trippable into that pipeline without
a translation layer.
Wiring this into PdfIngestOp itself (replacing the NDJSON/ReqIfCandidate
subprocess contract) is left as a follow-up — that changes an existing,
tested subprocess CLI contract and deserves its own pass rather than
being folded into this one.
201 existing ledger-core tests still pass; 12 new tests added.
…Service Adds ledgrrr-nats-service (feature-gated: nats-service), a real async-nats (0.45, service feature) Micro Service registering as 'ledgrrr' with endpoint 'classify' (subject ledgrrr.classify). This is the actual 'shared ledgrrr' — not the reqif-opa-mcp Docling tool it depends on, but ledgrrr's own deterministic bank_statement classifier made network-reachable and discoverable via 'nats service list', the Rust-side counterpart to reqif-opa-mcp's Python nats_docling_service.py (reqif-opa-mcp#22). Verified end-to-end against a real NATS test server: 'ledgrrr' shows up in 'nats service list' alongside 'ledgrrr-docling'; a request built from a real Docling extraction of the OWASP ASVS PDF plus synthetic bank-statement-shaped nodes is correctly classified (Unclassified for the real ASVS prose, TransactionRow/StatementHeader for the synthetic rows) and bridged into real TransactionInput/StatementHeader values. Intended chaining: extract via ledgrrr-docling's ledgrrr.extract, then classify+bridge via this service's ledgrrr.classify — two independently discoverable, independently deployable NATS services.
2 tasks
elasticdotventures
added a commit
that referenced
this pull request
Aug 23, 2026
… bridge (#200) * fix(ledger-core): PdfIngestOp actually calls a real command now, real 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. * fix(ledger-core): update pdf_ingest_integration fixtures for new PdfIngestOp 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.
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
docling_bridge: typed Rust mirror ofreqif-opa-mcp'sDocumentGraph/DocumentNode/SourceAnchorJSON schema — matches what bothreqif_ingest_cli extractand the newledgrrr-doclingNATS Micro Service (reqif-opa-mcp#22) actually return. Supersedesrule_registry::DocumentChunk, which was declared but never deserialized anywhere and used the wrong anchor shape.bank_statement: procedural (regex, never LLM)NodeCategoryclassifier overDoclingNodeviaufo_types::Satisfies<CategoryConstraint>, plusnode_to_transaction_inputbridging classified rows into the existingTransactionInput.NodeCategory::sarif_subtypes()mirrorsreqif-opa-mcp's SARIFproperties.subtypestagging convention, so this is round-trippable into that pipeline without a translation layer.Bug fixed
PdfIngestOp::executecurrently buildsTransactionInput { date: candidate.section, amount: candidate.confidence.to_string(), .. }from aReqIfCandidate— a modal-verb-detected requirement sentence, which has no real transaction date or dollar amount. It type-checks but is semantically garbage (a confidence score isn't a dollar amount). This PR adds the real bridge; wiring it intoPdfIngestOpitself (replacing the NDJSON/ReqIfCandidatesubprocess contract) is left as a deliberate follow-up rather than changing that existing tested contract in the same pass.Test plan
date: "05/01",amount: "-120.00") in place of the placeholder garbage described above.ledger-coresuite: 201 passed, 0 failed, 0 regressions.