ENG-181: Deal-related and fee-related cash event integration (#253)#255
Merged
Connorbelez merged 1 commit intomainfrom Mar 22, 2026
Merged
ENG-181: Deal-related and fee-related cash event integration (#253)#255Connorbelez merged 1 commit intomainfrom
Connorbelez merged 1 commit intomainfrom
Conversation
ENG-152 added paymentFrequency as a required third parameter but the caller in calculateServicingSplit was not updated, causing a runtime crash on any obligation settlement triggering dispersal. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements end-to-end posting for deal- and fee-related cash events, associates journal entries with deals, fixes account normal-balance classifications, and updates a servicing-fee calculation to pass the required paymentFrequency parameter to avoid runtime crashes. Sequence diagram for deal buyer funds received cash postingsequenceDiagram
actor CashSystem
participant Integrations as DealCashIntegrations
participant Posting as PostingPipeline
participant Idem as IdempotencyStore
participant Ledger as JournalEntryStore
CashSystem->>Integrations: postDealBuyerFundsReceived(dealId, cashEventId, amount, currency)
Integrations->>Idem: checkKeyExists(cashEventId)
Idem-->>Integrations: exists?
alt first_time_event
Integrations->>Posting: buildAndPostEntry(dealId, debitAccount TRUST_CASH, creditAccount CASH_CLEARING, amount)
Posting->>Ledger: createJournalEntry(dealId, debitAccount, creditAccount, amount, metadata)
Ledger-->>Posting: journalEntryId
Posting-->>Integrations: postingResult
Integrations->>Idem: storeKey(cashEventId, journalEntryId)
Idem-->>Integrations: stored
Integrations-->>CashSystem: postingResult
else duplicate_event
Integrations-->>CashSystem: existingPostingResult
end
Entity relationship diagram for journal entries associated to dealserDiagram
Deal {
string id
string externalId
string status
}
JournalEntry {
string id
string dealId
string debitAccountId
string creditAccountId
decimal amount
string currency
string metadata
datetime createdAt
}
Deal ||--o{ JournalEntry : has
Class diagram for cash event integrations, journal entries, and servicing fee calculationclassDiagram
class DealCashIntegrations {
+postDealBuyerFundsReceived(dealId, cashEventId, amount, currency)
+postDealSellerPayout(dealId, cashEventId, amount, currency)
+postLockingFeeReceived(dealId, cashEventId, amount, currency)
+postCommitmentDepositReceived(dealId, cashEventId, amount, currency)
-buildIdempotencyKey(cashEventId)
}
class PostingPipeline {
+postCashMovement(dealId, debitAccountCode, creditAccountCode, amount, currency, metadata)
}
class JournalEntry {
+string id
+string dealId
+string debitAccountCode
+string creditAccountCode
+decimal amount
+string currency
+string metadata
+datetime createdAt
}
class IdempotencyStore {
+boolean hasKey(idempotencyKey)
+string getResult(idempotencyKey)
+void saveResult(idempotencyKey, postingResult)
}
class AccountFamilyMap {
+boolean isCreditNormal(accountCode)
+void setCreditNormal(accountCode, isCreditNormal)
}
class ServicingCalculator {
+calculateServicingSplit(servicingConfig, mortgage, settledAmount)
+calculateServicingFee(annualRate, principal, paymentFrequency)
}
class ServicingConfig {
+decimal annualRate
}
class Mortgage {
+decimal principal
+string paymentFrequency
}
DealCashIntegrations --> PostingPipeline : uses
DealCashIntegrations --> IdempotencyStore : uses
PostingPipeline --> JournalEntry : creates
AccountFamilyMap --> JournalEntry : validatesAccounts
ServicingCalculator --> ServicingConfig : uses
ServicingCalculator --> Mortgage : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Connorbelez has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- When passing
args.mortgage.paymentFrequencyintocalculateServicingFee, consider handling or validating unexpected/legacy values (includingundefined/null) to avoid new runtime failures where older records may not have this field populated. - If
paymentFrequencyhas a constrained set of allowed values, it may be helpful to reflect that in the type ofargs.mortgageso that incorrect values are caught at compile time rather than during fee calculation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When passing `args.mortgage.paymentFrequency` into `calculateServicingFee`, consider handling or validating unexpected/legacy values (including `undefined`/`null`) to avoid new runtime failures where older records may not have this field populated.
- If `paymentFrequency` has a constrained set of allowed values, it may be helpful to reflect that in the type of `args.mortgage` so that incorrect values are caught at compile time rather than during fee calculation.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Updates the dispersal pipeline’s servicing-fee calculation to use the mortgage’s payment frequency, aligning the call site with the updated calculateServicingFee API and preventing runtime failures during obligation settlement/dispersal.
Changes:
- Passes
mortgage.paymentFrequencyintocalculateServicingFeewhen computing servicing fee due.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
…255) ENG-181: Deal-related and fee-related cash event integration (#253) ## Wire 4 deal/fee cash events through the posting pipeline - postDealBuyerFundsReceived (CASH_RECEIVED → TRUST_CASH/CASH_CLEARING) - postDealSellerPayout (LENDER_PAYOUT_SENT → LENDER_PAYABLE/TRUST_CASH) - postLockingFeeReceived (CASH_RECEIVED → TRUST_CASH/UNAPPLIED_CASH) - postCommitmentDepositReceived (CASH_RECEIVED → TRUST_CASH/UNAPPLIED_CASH) Also fixes CREDIT_NORMAL_FAMILIES to include CASH_CLEARING and UNAPPLIED_CASH — these are liabilities (credit-normal) not assets, which corrects balance computation polarity for all future entries. ### Changes - Adds 4 new integration functions in `integrations.ts` with proper idempotency keys and metadata - Expands CASH_RECEIVED family map to accept CASH_CLEARING and UNAPPLIED_CASH as credit accounts - Adds dealId field to journal entries schema with corresponding index - Includes comprehensive test coverage for all functions including idempotency and error handling - Corrects account normal balance classifications for proper accounting semantics ## Summary by Sourcery Bug Fixes: - Fix runtime crash in obligation settlement dispersal caused by missing paymentFrequency argument in servicing fee calculation. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added cash posting functions for deal-related transactions: buyer funds received, seller payouts, locking fees, and commitment deposits. * Added support for associating cash ledger entries with specific deals for better transaction tracking. * **Tests** * Comprehensive test coverage for new cash posting functions, including idempotency and error handling scenarios. * **Bug Fixes** * Corrected cash account balance preconditions in existing tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ENG-224: Pass paymentFrequency to calculateServicingFee ENG-152 added paymentFrequency as a required third parameter but the caller in calculateServicingSplit was not updated, causing a runtime crash on any obligation settlement triggering dispersal. Co-Authored-By: Claude Opus 4.6 (1M context) <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
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.
ENG-181: Deal-related and fee-related cash event integration (#253)
Wire 4 deal/fee cash events through the posting pipeline
Also fixes CREDIT_NORMAL_FAMILIES to include CASH_CLEARING and UNAPPLIED_CASH — these are liabilities (credit-normal) not assets, which corrects balance computation polarity for all future entries.
Changes
integrations.tswith proper idempotency keys and metadataSummary by Sourcery
Bug Fixes:
Summary by CodeRabbit
New Features
Tests
Bug Fixes
ENG-224: Pass paymentFrequency to calculateServicingFee
ENG-152 added paymentFrequency as a required third parameter but the
caller in calculateServicingSplit was not updated, causing a runtime
crash on any obligation settlement triggering dispersal.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com