Skip to content

feat: Add AR/AP duality support with event handlers and ledger reads#676

Merged
jfrench9 merged 3 commits into
mainfrom
feature/ar-ap-duality
May 15, 2026
Merged

feat: Add AR/AP duality support with event handlers and ledger reads#676
jfrench9 merged 3 commits into
mainfrom
feature/ar-ap-duality

Conversation

@jfrench9
Copy link
Copy Markdown
Member

Summary

Introduces Accounts Receivable / Accounts Payable (AR/AP) duality handling across the platform, spanning data modeling, event processing, GraphQL APIs, and ledger read operations. This feature enables the system to properly track and reconcile the dual nature of AR/AP transactions — where a receivable or payable on one side corresponds to a payment or bill settlement on the other.

Key Accomplishments

Data Layer (dbt Models)

  • Enhanced the transactions.sql ledger model to account for AR/AP duality in transaction aggregation
  • Updated six QuickBooks staging models (bill_headers, bill_payment_headers, invoice_headers, payment_headers, purchase_headers, sales_receipt_headers) to surface fields needed for AR/AP tracking

API Models & GraphQL

  • Added new ar_ap.py API extension model defining the data structures for AR/AP duality
  • Exposed AR/AP duality data through new GraphQL types and resolvers in the ledger domain
  • Registered the new extension in the extensions module

Event Handlers

  • bill_paid handler — processes bill payment events, linking AP settlements to their originating bills
  • payment_received handler — processes incoming payment events, linking AR settlements to their originating invoices; includes comprehensive logic (~250 lines) for matching and reconciliation
  • Updated the handler registry to include both new handlers

Ledger Read Operations

  • Added ar_ap.py read module (~208 lines) providing query operations for AR/AP balances, aging, and duality lookups

Pipeline Utilities

  • Extended QuickBooks pipeline utilities to support AR/AP-specific data extraction and transformation

Extensions Loader

  • Updated the extensions loader to discover and wire up the new AR/AP extension

Rules Engine

  • Modified the information block rules engine to incorporate AR/AP duality awareness in rule evaluation

Breaking Changes

None expected. All changes are additive — new handlers, new models, new read operations, and new GraphQL types/resolvers. Existing staging models received additional columns but no removals or renames.

Testing

  • New test suites added:
    • test_bill_paid.py — 76 lines covering bill payment event handler scenarios
    • test_payment_received.py — 283 lines with comprehensive coverage of payment received logic including edge cases and reconciliation matching
    • test_ar_ap.py (reads) — 112 lines testing AR/AP ledger read operations
  • Existing tests updated:
    • test_utils.py — expanded with 118 additional lines for new pipeline utility functions
    • test_registry.py — updated to validate registration of the two new event handlers

Total new/modified test coverage: ~600+ lines across 5 test files.

Infrastructure Considerations

  • The dbt model changes will require a dbt run on the affected staging and ledger models to materialize the new columns in the data warehouse
  • The new GraphQL types and resolvers will be available immediately upon deployment without schema migration, but clients should be updated to leverage the new AR/AP query capabilities
  • The event handler registry changes are backward-compatible; the new handlers will only fire for their respective event types

🤖 Generated with Claude Code

Branch Info:

  • Source: feature/ar-ap-duality
  • Target: main
  • Type: feature

Co-Authored-By: Claude noreply@anthropic.com

- Added BILL_PAID_HANDLER and PAYMENT_RECEIVED_HANDLER to the event block registry for handling bill payments and received payments respectively.
- Implemented linked transaction parsing in OLTPLoader to capture invoice/bill references from payments.
- Introduced new AR/AP read operations to compute open balances and list receivables/payables by agent.
- Developed tests for the new bill_paid and payment_received handlers, ensuring correct linkage and dispatch behavior.
- Added smoke tests for AR/AP read operations to validate SQL compilation and response shapes.
@jfrench9
Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude
Copy link
Copy Markdown

claude Bot commented May 15, 2026

Claude finished @jfrench9's task in 2m 41s —— View job


PR Review: feat: Add AR/AP duality support with event handlers and ledger reads

Tasks

  • Gather context and understand request
  • Read changed files
  • Review data models and API layer
  • Review event handlers
  • Review ledger reads and GraphQL
  • Review tests
  • Post final review

Overall Assessment

This is a well-designed, well-documented PR. The AR/AP duality approach — using an event-graph model where discharges_event_id links settlements back to originating obligations — is a clean design that avoids GL roll-up complexity. The code is idiomatic, properly layered through the ops kernel, and the tests cover the critical logic paths. I found no bugs, but there are a few items worth discussing before merge.


Issues to Address

1. Behavior Change in Rules Engine is Bundled in this PR

robosystems/operations/information_block/rules/engine.py removes the AND fact_scope = 'in_scope' filter from _bind_sum_variables. This is a non-trivial semantic change (it switches from filtering to only in-scope facts to summing all periodic facts on the structure). It's not AR/AP-related and deserves its own commit at minimum, or ideally a separate PR with an explicit description of what schedules were previously failing. As written, the change could silently fix or break existing SumEquals rule evaluations.

Fix this →

2. Discharge Target Candidates Not Filtered by Status

In payment_received.py, both _resolve_by_linked_txns (line 87–96) and _resolve_by_reference_number (lines 118–131) query originating events without a status filter. A captured or classified invoice — one that hasn't been committed yet — can be set as the discharge target.

For most QB syncs this is benign (QB syncs are sequential so invoices precede payments in the event store). But during a fresh full re-sync where auto-commit runs in external_id order, a payment may be processed before its invoice has been committed. The discharge link would be set to a captured event. If that invoice is later voided or superseded, the link is stale.

# In _resolve_by_linked_txns — consider adding:
Event.status.in_(("committed", "fulfilled")),

Fix this →

3. NULL agent_id Silently Becomes Empty String in API Response

In ar_ap.py line 161:

agent_id=str(r.agent_id) if r.agent_id else "",

Events with no agent (e.g., cash-basis income with no AR cycle that somehow lands in the aggregate) would return agent_id: "" to callers. The OpenBalanceByAgent model declares agent_id: str (not str | None), so this is the only option, but "" is an opaque sentinel that callers cannot distinguish from a real empty-string agent id.

Consider either making OpenBalanceByAgent.agent_id: str | None and serializing NULL as null, or defining a UNSCOPED_AGENT_ID constant. The current code is functional but could surprise consumers.

Fix this →


Minor Observations

4. No Pagination on open_*_by_agent GraphQL Fields

open_receivables_by_agent and open_payables_by_agent in ledger.py (lines 252–274) return unbounded list[OpenBalanceByAgent]. For a book with hundreds or thousands of counterparties, this could be a large payload. Since this is v1 and the PR description doesn't mention large books, this is fine to ship — but worth adding a limit/offset before the field goes into a production dashboard.

5. _AR_ORIGINATING_TYPES Defined in Two Modules

payment_received.py line 212 and ar_ap.py line 36 both define:

_AR_ORIGINATING_TYPES: tuple[str, ...] = ("invoice_issued", "sales_receipt_recorded")

Likewise _AP_ORIGINATING_TYPES appears in both bill_paid.py and ar_ap.py. These are intentionally kept private per module (the reads layer shouldn't import from the handler layer), but a shared constants.py in the roboledger domain would be worth considering if a third consumer appears.

6. purchase_headers Staging Models Pick Up linked_txns Correctly

The stg_qb_purchase_headers.sql change adds cast("linked_txns" as varchar), and flatten_purchase_headers always emits "linked_txns": "[]". The UNION ALL in all_headers is therefore shape-consistent. ✓


What's Well Done

  • Handler design: Delegating GL writes to journal_dispatch and adding the discharge step after is clean. The idempotency guard in _link_discharge (early-return when discharges_event_id is already set) is the right call.
  • Two-stage resolution: LinkedTxn-first with reference_number fallback mirrors QB's own data model. The priority ordering is correct.
  • SQL for open balance: The self-join approach (originating LEFT JOIN discharges ON discharges.discharges_event_id = originating.id) is correct and avoids GL roll-up. The _build_open_balance_subquery reuse for both aggregate and per-agent reads avoids drift.
  • _raise_ledger_not_initialized() used from None: Prevents Strawberry from leaking ProgrammingError details (schema/table names) through the extensions field. The existing resolvers follow this pattern and the new ones do too. ✓
  • Agent-level AR/AP fields: Adding open_receivable / open_payable directly to the Agent Strawberry type with a clear N+1 warning is the right UX tradeoff for v1 drill-in flows.
  • Test structure: test_payment_received.py covers all meaningful branches of the two-stage matching logic. The SQL compilation test in test_ar_ap.py catches column-name bugs without needing a live database.
  • Multi-invoice split payment acknowledged: The docstring in payment_received.py (lines 28–35) explicitly names the v1 limitation and describes what's queryable for a future per-line linkage table. This is the right documentation approach.

Summary: Two issues worth addressing before merge — the bundled rules engine behavior change (#1) and the missing status filter on originating event lookups (#2). The NULL agent_id serialization (#3) is a UX concern but not a blocker. Everything else is clean.

@jfrench9 jfrench9 merged commit dba5ed3 into main May 15, 2026
7 checks passed
@jfrench9 jfrench9 deleted the feature/ar-ap-duality branch May 15, 2026 19:43
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.

1 participant