Skip to content

FINERACT-2455: Rework WC loan schedule model to use recurrent calculations - #6369

Merged
adamsaghy merged 2 commits into
apache:developfrom
openMF:FINERACT-2455-rework-amortization-schedule-model
Sep 7, 2026
Merged

FINERACT-2455: Rework WC loan schedule model to use recurrent calculations#6369
adamsaghy merged 2 commits into
apache:developfrom
openMF:FINERACT-2455-rework-amortization-schedule-model

Conversation

@Cocoa-Puffs

Copy link
Copy Markdown
Contributor

Description

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per our guidelines
  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
  • Create/update unit or integration tests for verifying the changes made.
  • Follow our coding conventions.
  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

@Cocoa-Puffs Cocoa-Puffs changed the title FINERACT-2798: Rework WC loan schedule model to use recurrent calculations FINERACT-2455: Rework WC loan schedule model to use recurrent calculations Sep 2, 2026
@Cocoa-Puffs
Cocoa-Puffs force-pushed the FINERACT-2455-rework-amortization-schedule-model branch 4 times, most recently from b3ead3d to 6668012 Compare September 3, 2026 14:07
@Cocoa-Puffs
Cocoa-Puffs marked this pull request as ready for review September 3, 2026 16:55
@adamsaghy

Copy link
Copy Markdown
Contributor

@Cocoa-Puffs Can you please review the below?

  1. Persisted rate changes are silently dropped, then persisted away. rateSegments is replaced by rateChanges, and ProjectedAmortizationScheduleRepositoryWrapperImpl.readModel never looks at jsonModelVersion — the bump to "7" has no effect on reads. So a v6 model that carried rate segments loads with rateChanges empty and rebuilds at the original rate. The new parser test says the schedule "is restated from that table the next time anything writes the loan", but applyRepayment (WorkingCapitalLoanAmortizationScheduleWriteServiceImpl:294) does not replay from m_wc_loan_period_payment_rate_change — it reads, applies the payment, and writes the now-rate-less model back, baking the loss in. Only regenerateAmortizationScheduleOnRateChange replays. Progressive loans gate this with findLoanIdsRequiringModelRecalculation; WC has no equivalent. If no released deployment has WC loans with rate changes this is moot — worth saying so in the PR either way.

  2. On a schedule whose balance has closed, each further acknowledgeElapsedPeriods appends a zero row and moves the maturity date. Repay in full on day 1, then acknowledge 120 days: base stays at 3 rows with scheduledMaturityDate = 2026-01-03; the PR grows to 121 rows with scheduledMaturityDate = 2026-05-01, and writeModel pushes that into loan.expectedMaturityDate on every write. minimumScheduleDays() is elapsedPeriodCount() + 1 with no "already closed" exit. COB's loan selection excludes closed and overpaid loans, so this needs a loan whose principal has closed while it stays ACTIVE (outstanding charges/penalties) to bite — but it is a behavior change from base worth a deliberate answer.

  3. Residual, not a blocker: after the borrower deviates from plan, the static projection still doesn't sum to what is owed. E.g. 3030/970/22 with four small payments: last known row shows 3614.00 owed, the remaining rows bill 3613.91. The gap is the fee that aggregatedHighPrecisionExpected = discountFee (AmortizationWalk:582) completes on the closing day without billing it. It is smaller than base (which was +1.70 on the same input) and self-corrects as payments land — but "the schedule accounts for the full payable" holds exactly only for the single-tail-row case in the ticket.

No 3 in details:

The residual cent gap in the WC projection

This note is about a smaller residue that survives it: after the
borrower has deviated from plan, the forward projection still sums to a few cents less than what
is owed. Measured on the PR branch, not inferred.

The measurement

3030/970/22, payments 124@d4, 108@d8, 12@d10, 142@d12   (daily = 61.11, eir = 0.0088086)

lastKnown row (day 12):  actBal = 2807.40   actFeeBal = 806.60   -> owed 3614.00

tail (60 rows):  sum(expectedPaymentAmount)       = 3613.91    GAP vs owed = 0.09
                 sum(expectedAmortizationAmount)  =  806.60    (= the unearned fee, exactly)
                 accrual implied by the recursion =  806.51
                 reported fee - accrued fee       =    0.09    <- the same 0.09

What the two sides are

"Owed" is exact by construction. actualBalance = net − collected + aggNormActual and
actualDiscountFeeBalance = fee − aggNormActual. Add them and aggNormActual cancels:
net + fee − collected = 4000 − 386 = 3614.00. It cannot be wrong.

"Billed" is governed by the balance recursion. Each tail row does
grown = balance × (1 + eir), bill = min(daily, grown), balance = grown − bill, ending at zero.
Telescope that and you get an identity with no slack in it:

Σ bills  =  startBalance  −  0  +  Σ accruals

So the tail bills what is owed only if its own accruals sum to exactly the fee still
unearned — 806.60 here.

Why they don't match

eir is solved once, as the IRR of the cash flow [−3030, 61.11 × 65, 27.85]. On that path the
accruals sum to exactly 970. This borrower paid 386 over 12 days instead of the ~733 the plan
assumed, so the balance the tail runs on is not the balance the rate was solved for, and its 60 days
of accrual come to 806.51 rather than 806.60.

AmortizationWalk.java:582aggregatedHighPrecisionExpected = discountFee on the closing day —
resolves that by fiat on the fee column: the deferred balance closes on 0.00 and the tail's
reported amortization sums to 806.60. But expectedPaymentAmount comes from min(daily, grown),
which knows nothing about the forced completion, so the billing column still reflects only
806.51 of accrual. The 0.09 the fee column gains is money no row ever asks for.

It is visible on the closing row:

row before closing:  bill = 61.11   expAmort = 0.60   expBal = 8.34
closing row:         bill =  8.42   expAmort = 0.17   expBal = 0.00   expFeeBal = 0.00

8.34 × eir ≈ 0.07 is what that day actually accrues; the reported 0.17 is that plus the 0.09 jump.

Why the ticket's own scenario shows 0.00

Not because it has a single tail row.

ticket case (9000/1000/17), lastKnown row (day 3):  actBal = 4.00   actFeeBal = 0.00
tail (1 row):  bill = 4.00   expAmort = 0.00   GAP = 0.00

The fee is already fully earned at the last known dayaggHpActual reaches ≈ 999.995 and
normalizes to 1000.00 — so actFeeBal is 0.00 before the tail begins. There is nothing left for the
closing clause to force, the jump is zero, and no gap can open. (A sub-cent residue does remain: the
tail's exact bill is 3.9994 against 4.00 owed, absorbed by rounding to 4.00.)

So the ticket's third acceptance line is met for a reason that does not generalise — it needed the
fee to be complete already, not the tail to be short.

Why this is not a blocker

  • It is confined to the forward projection. Every settled day re-bases from
    net − collected + aggHpActual and re-reads the fee off PlanCursor; neither touches the
    projection's accruals. A borrower paying exactly what each day's schedule bills closes on exactly
    the payable — verified for four shapes of loan, all four landing on 10000.00 / 4000.00. The 0.09
    is not lost money: today's forecast of the sum of future bills is 0.09 under what will actually be
    billed once those days arrive and re-bill.
  • It is a large improvement on what it replaces. On the same input the base
    (a6818b556a) projection was off by +1.70 (3615.70 against 3614.00) — roughly twenty times
    worse. I did not diagnose base's mechanism, since that code is removed by this PR.

Worth raising anyway

The ticket's acceptance reads "the schedule accounts for the full 10000". On any schedule where the
borrower has deviated and fee remains unearned at the last known day, it accounts for a few
cents less.

adamsaghy
adamsaghy previously approved these changes Sep 4, 2026

@adamsaghy adamsaghy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Cocoa-Puffs
Cocoa-Puffs force-pushed the FINERACT-2455-rework-amortization-schedule-model branch from 3305fcc to 130fa13 Compare September 7, 2026 10:36
@Cocoa-Puffs
Cocoa-Puffs force-pushed the FINERACT-2455-rework-amortization-schedule-model branch from 130fa13 to a1e3c12 Compare September 7, 2026 10:36
@Cocoa-Puffs

Copy link
Copy Markdown
Contributor Author

I've addressed all 3 concerns:

1. Claim: persisted rate changes silently dropped, then persisted away

This was unfortunately true. Currently there is no similar mechanism for WC loans that exists for term loans in regards to regenerating loans with outdated model versions. I have implemented the same functionality that we have for progressive loans.

# Piece Location
1 Version queries ProjectedAmortizationLoanModelRepository -> existsByLoanIdAndJsonModelVersionNot, findLoanIdsRequiringModelRecalculation
2 Processing service WorkingCapitalLoanModelProcessingService (new) -> requiresModelRecalculation, findLoanIdsRequiringModelRecalculation, recalculateModelAndSave in REQUIRES_NEW
3 Rebuild entry point rebuildScheduleModelFromRecordedHistory on the write service
4 COB hook AbstractWorkingCapitalLoanCOBWorkerItemProcessor.process, rebuilding before business steps; constructors threaded through both concrete processors and both configs
5 API filter WorkingCapitalLoanModelCheckerFilter + WorkingCapitalLoanModelCheckerHelper (new), wired in SecurityConfig

2. Claim: closed schedules grow a zero row per elapsed day and move the maturity date

This was a real bug that is now fixed.

Where it failed: ProjectedAmortizationScheduleModel.minimumScheduleDays()

int minimum = elapsedPeriodCount() + 1;   // the calendar, not recorded facts

That floor only ever bit on a loan whose balance closes before today. While a loan still owes something the walk reaches today anyway, it bills until the balance closes and an unpaid day doesn't bring that closer.

The fix:

int minimum = 1;
final int offset = currentFirstPeriodDayOffset();
for (final ActualPayment payment : actualPayments) {
    minimum = Math.max(minimum, resolvePaymentIndex(payment.date(), offset) + 1);
}
for (final PrincipalAdjustment adjustment : principalAdjustments) {
    minimum = Math.max(minimum, resolvePaymentIndex(adjustment.date(), offset) + 1);
}

Not "exit when closed": a payment dated after the closing day currently reaches the schedule only because the elapsed floor drags the walk out to it. A naive closed-check would silently drop it. scheduleTerm() still carries elapsedPeriodCount() + 1 and is untouched, it governs the valid date range for a payment or rate change, which should extend to today.

New unit test validating the behaviour: aClosedScheduleGrowsToReachMoneyButNotMerelyToReachToday

3. Claim: residual gap in the forward projection

This is now fixed. Three details, each load-bearing:

  1. Split the rates. dailyPayment = TPV × rate / npvDayCount / 100 has no balance term, so re-solving at the same period rate returns the same instalment — only eir and term move. The projection can therefore have its own rate while PlanCursor keeps the contractual one. Recognised income is untouched by construction.

  2. Pair exact with exact. Using discountFee − aggregatedHighPrecisionActual makes the sum exactly net + fee − collected, so solving from a position the plan already predicted returns the rate it already had: the re-solve is a no-op for an on-time payer.

  3. Re-solve lazily. Days with a record are always a run from the start, so one solve from the last of them re-prices the whole tail:

if (projectionStale && !settled) {
    final BigDecimal unearnedFee = discountFee.subtract(aggregatedHighPrecisionActual, mc);
    if (balance.signum() > 0 && unearnedFee.signum() > 0) {
        try {
            projection = AmortizationParams.solve(balance, unearnedFee, totalPaymentVolume, rateInForce, ...);
        } catch (final IllegalArgumentException | IllegalStateException | ArithmeticException e) {
            log.debug("Could not re-price the projection from balance {} with {} of fee unearned", balance, unearnedFee, e);
        }
    }
    projectionStale = false;
}

5,705 → 470 solves, output bit-for-bit identical to solving on every settled day, and the cost problem of resolving for every day disappears.

@mariiaKraievska

Copy link
Copy Markdown
Contributor

@Cocoa-Puffs I added a set of additional WC amortization e2e scenarios to stress the rework the other way around: mutate the loan, then force a near-payoff / backdated large repayment and assert exact totals (fee earned and payable to the cent, no silent drift). Coverage includes:

  • near-payoff → rate
  • discount adjustment → backdated near-payoff (alone, with rate change, same-day)
  • discount adjustment after near-payoff
  • two rate changes → near-payoff
  • COB catch-up / undo-after-COB / backdated rate-after-COB → near-payoff
  • mid-term charge not disturbing WC payable near-payoff

сс @ruzeynalov

@adamsaghy adamsaghy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamsaghy
adamsaghy merged commit 0d295e8 into apache:develop Sep 7, 2026
91 checks passed
@adamsaghy
adamsaghy deleted the FINERACT-2455-rework-amortization-schedule-model branch September 7, 2026 12:37
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.

3 participants