adamsaghy commented on PR #6369:
URL: https://github.com/apache/fineract/pull/6369#issuecomment-5540784188
@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:582` — `aggregatedHighPrecisionExpected =
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 day** — `aggHpActual`
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
