feat(prd-7): populate TransactionFacts from PipelineState doc_fields (#55)#90
Merged
Merged
Conversation
Adds DocumentFields to PipelineState<S> so to_transaction_facts() emits non-None fields, making verify_legal() produce real Z3 signal instead of Unknown on every rule. Closes #55. - Add DocumentFields struct (vendor_jurisdiction, supply_type, tax_code, amount as Decimal, is_business_activity, is_ordinary, is_necessary) - Add doc_fields field + with_doc_fields() builder to PipelineState<S> - Thread doc_fields through all 5 state-transition constructors - Fix to_transaction_facts() to map doc_fields into TransactionFacts - Replace placeholder test_verify_legal_integration with two real scenarios: BASEXCLUDED passes au_gst::rule_38_190, INPUT fails it - Add From<&TransactionInput> for DocumentFields in ingest.rs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends ledger-core’s typed pipeline state to carry structured document-derived fields, so the legal verification stage can emit meaningful TransactionFacts (instead of always passing fact-free defaults that lead to Z3Result::Unknown).
Changes:
- Added
DocumentFieldsand threaded it throughPipelineState<S>construction and all state transitions. - Implemented
PipelineState<Validated>::to_transaction_facts()to populateTransactionFactsfromdoc_fields. - Replaced the placeholder legal integration test with two concrete scenarios for
au_gst::rule_38_190().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/ledger-core/src/pipeline.rs | Adds DocumentFields to pipeline state and maps it into TransactionFacts; updates legal integration tests accordingly. |
| crates/ledger-core/src/ingest.rs | Adds From<&TransactionInput> for DocumentFields (currently populating amount). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub source_ref: String, | ||
| pub confidence: f32, | ||
| pub issues: Vec<crate::validation::Issue>, | ||
| pub meta: crate::validation::MetaCtx, |
Comment on lines
+669
to
+679
| // US SaaS with BASEXCLUDED → legal gate passes → Ok(Classified) | ||
| let state = PipelineState::<Ingested>::new("doc1", "WF--BH--2026-01") | ||
| .with_doc_fields(DocumentFields { | ||
| vendor_jurisdiction: Some("US".to_string()), | ||
| supply_type: Some("SaaS".to_string()), | ||
| tax_code: Some("BASEXCLUDED".to_string()), | ||
| ..DocumentFields::default() | ||
| }) | ||
| .validate(Vec::new()); | ||
| assert!(state.verify_legal(&solver, &rules).is_ok()); | ||
|
|
Comment on lines
+680
to
+689
| // US SaaS with INPUT → legal gate fails → Err(NeedsReview) | ||
| let state = PipelineState::<Ingested>::new("doc2", "WF--BH--2026-01") | ||
| .with_doc_fields(DocumentFields { | ||
| vendor_jurisdiction: Some("US".to_string()), | ||
| supply_type: Some("SaaS".to_string()), | ||
| tax_code: Some("INPUT".to_string()), | ||
| ..DocumentFields::default() | ||
| }) | ||
| .validate(Vec::new()); | ||
| assert!(state.verify_legal(&solver, &rules).is_err()); |
Comment on lines
+112
to
+113
| Self { | ||
| amount: row.amount.trim().parse().ok(), |
Member
|
@copilot apply changes based on the comments in this thread perform detailed comprehensive fix. |
…al assertions Agent-Logs-Url: https://github.com/PromptExecution/ledgrrr/sessions/fbd0329a-cc5e-411b-9a4e-89ae31919880 Co-authored-by: elasticdotventures <35611074+elasticdotventures@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PromptExecution/ledgrrr/sessions/fbd0329a-cc5e-411b-9a4e-89ae31919880 Co-authored-by: elasticdotventures <35611074+elasticdotventures@users.noreply.github.com>
Contributor
Implemented the full review-thread fixes in commits 535d33d and def2f0f: added |
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
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
DocumentFieldsstruct toPipelineState<S>soto_transaction_facts()emits non-None fields — legal gate now produces real Z3 signal instead ofUnknownon every ruledoc_fieldsthrough all 5 state-transition constructors (validate,verify_legalboth arms,classify,reconcile,request_review)test_verify_legal_integrationwith two real AC scenarios:BASEXCLUDEDpassesau_gst::rule_38_190,INPUTfails itFrom<&TransactionInput> for DocumentFieldsiningest.rs(amount parsed via.parse().ok(), no unwrap)Closes #55. Unblocks PRD-7 Phase 2 (Z3 upgrade) and Phase 3 (audit sheet).
Test plan
cargo test -p ledger-core— all 152 tests passtest_verify_legal_integration— BASEXCLUDED→Ok(Classified), INPUT→Err(NeedsReview)againstau_gst::rule_38_190@copilot please review
🤖 Generated with Claude Code