Skip to content

Fix schedule generation, APR calculations, and rebate handling - #71

Open
Thorium wants to merge 18 commits into
simontreanor:mainfrom
Thorium:integration-critical-fixes
Open

Thorium wants to merge 18 commits into
simontreanor:mainfrom
Thorium:integration-critical-fixes

Conversation

@Thorium

@Thorium Thorium commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

One of my background agents (Fable 5) was checking critical issues with this library.

This PR hast the findings fixed, and here is a Copilot generated text-wall as descriptions:

This pull request introduces several important updates and fixes to the amortisation and APR calculation logic, with a focus on improving accuracy, regulatory compliance, and robustness in edge cases. The main areas of change are in the amortisation schedule generation, statutory fee rebate handling, APR calculation, and payment/refund classification.

Amortisation and Fee Rebate Calculation Improvements:

  • Improved statutory fee rebate handling for UK FCA-regulated agreements, ensuring the higher of the calculated or statutory rebate is used, and correctly apportioning excess rebates between fee and principal balances. [1] [2] [3]
  • Added guards and improved handling for edge cases in amortisation, such as empty payment schedules and schedules with only rescheduled payments, to prevent calculation errors. [1] [2] [3]
  • Enhanced logic for suppressing or applying charges on days with no scheduled payment due, ensuring failed payment charges are still applied when appropriate.
  • Refined the way charges and interest are added to scheduled payments under the AddChargesAndInterest option, ensuring the payment due does not exceed the outstanding balance.

APR Calculation and Unit Period Handling:

  • Corrected the calculation of unit periods for daily and weekly schedules to use actual days between dates rather than assuming consecutive periods, improving accuracy for irregular schedules. [1] [2]
  • Improved the robustness of the APR calculation by handling edge cases such as all transfers on the same day, and by catching overflow errors in the divisor during rate calculation. [1] [2]
  • Updated the roughUnitPeriodRate to return a decimal rate (not a percentage) for use as the solver’s initial guess.

Payment/Refund Classification Logic:

  • Refined the classification of payments and refunds, including handling refunds issued when a payment was due and ensuring partial payments on the settlement day are correctly netted against the generated settlement. [1] [2]

Minor Adjustments and Consistency:

  • Updated calls to adjustFinalPayment to pass the correct parameters for consistency and correctness. [1] [2]
  • Minor code cleanups and improved comments for clarity.

These changes collectively enhance the accuracy, reliability, and regulatory compliance of the amortisation and APR calculation modules, especially in handling edge cases and UK-specific requirements.

Thorium and others added 18 commits July 8, 2026 12:35
- reschedule: an AutoGenerateSchedule payment plan was emitted as original
  payments with no reschedule day, so merging never cancelled the old
  payments and the customer owed both schedules; the level payment was
  also solved against the full original principal. The plan is now
  generated against the outstanding balance on the evaluation day and
  re-based onto the original schedule's day axis as rescheduled payments,
  mirroring the fixed-schedules rescheduling path.
- rollOver: the pro-rata fee rebate was given the original final payment
  day, an offset from the original start date, but the new schedule's
  day 0 is the rollover date, so the rebate window overran by the elapsed
  term. The day is now re-based onto the new schedule's axis, with an
  expired window meaning no rebate is due.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- nearest: average the normalised period lengths instead of their
  occurrence counts (which were always 1 in this branch, so every
  irregular schedule was detected as daily), and fall back to a daily
  unit-period when there are no intervals at all instead of throwing
  on an empty average
- detect: derive the second semi-monthly tracking day from a single
  transfer date instead of throwing an index-out-of-range exception
- Config.constrain: validate the month of monthly and semi-monthly
  configs (repairing out-of-range values as with days) and reject
  non-positive weekly/monthly multiples with a descriptive message,
  as these would otherwise generate unbounded schedules
- generatePaymentSchedule: fix reverse max-duration generation for
  daily, weekly and monthly configs, which iterated forwards with an
  inverted window comparison (yielding empty or out-of-window
  schedules), to mirror the correct semi-monthly implementation

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- generalEquation: wire up the single advance, single payment rule
  ((b)(5)(vi)/(vii)): the unit-period is the term itself, with 12/months
  unit-periods per year for whole-month terms or 365/days otherwise
  (previously the dead singleUnitPeriod code was never routed to and
  such transactions were priced on a daily unit-period)
- dailyUnitPeriods/weeklyUnitPeriods: compute quotients and remainders
  from the actual transfer dates as required by (b)(5)(iv), instead of
  index-based counts that assumed consecutive unit-periods and made the
  APR insensitive to actual payment dates
- generalEquation: return Impossible for same-day transactions instead
  of crashing or reporting a meaningless rate
- calculateAprEuUk: guard the discounting against decimal overflow at
  rates near -1 with long-dated payments, as the US path already does
- roughUnitPeriodRate: return a decimal rate rather than a percentage,
  as the solver expects a rate for its initial guess (the 100x guess
  prevented convergence on high-APR schedules)
- toPercent: do not report the partial value of a non-converged
  solution as if it were a valid APR
- ukUnitPeriodRate: fail descriptively on a unit period with zero
  periods per year instead of throwing a divide-by-zero exception
- solveNewtonRaphson: catch decimal overflow when evaluating the
  objective or derivative and dampen the guess instead of crashing,
  and scale the derivative step relative to the guess so that the
  derivative does not vanish (through rounding) at very large rates

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- AprUsActuarialTestsExtra: Reg Z (b)(5)(vi)/(vii) single-payment
  examples, irregular two-payment schedule with derivation, and
  date-sensitivity of daily and weekly unit-period quotients
- PaymentScheduleTests: nearest unit-period averaging and fallback,
  semi-monthly detection from a single date, constrain repairs and
  rejections, and reverse max-duration schedule generation
  (placed here because UnitPeriodConfigTests.fs is excluded from the
  test project pending migration to the current scheduling API)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ong rebate

The UK statutory fee rebate calculation threw an empty-array exception
when a settlement was generated before the first original scheduled
payment day (or when the schedule contained only rescheduled payments).
The settlement part-period now falls back to the start of the agreement,
per the structure of the CCA 2004 regulation 4(1) formula.

The generated settlement figure was also composed using the pro-rata
rebate while the applied rebate was the (higher) statutory rebate,
over-quoting the customer and leaving the apportionment portions
inconsistent with the payment value. The applicable rebate is now
determined first and used for both the settlement figure and the fee
apportionment, with any rebate in excess of the fee balance reducing
the principal portion instead of producing a negative fee portion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s on scheduled days

A confirmed partial payment made on the settlement evaluation day itself
had its net effect zeroed, so the generated settlement figure did not
deduct it and the customer was quoted the full figure again. The
confirmed total now takes net effect on the quote day (full and
overpayments on the quote day already behaved correctly and are
unchanged, as is the payment-timeout grace behaviour).

A refund issued on a scheduled-payment day was classified as an
underpayment, incurring a late-payment charge and later being
mislabelled as paid-later-owing; it is now classified as a refund.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…chedule

An empty payment schedule (e.g. a custom schedule with no payments)
previously crashed with an unhelpful empty-sequence exception deep
inside the amortisation calculation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nterest

Under the AddChargesAndInterest option, the whole net effect was
assigned to the fee and principal balances while the interest and
charges portions were also recorded, so the portions summed to more
than the amount collected, and the scheduled-payment adjustment had a
sign error that corrupted the cumulative scheduled payments. The
adjustment now increases the payment due (and, for payments not yet
due, the net effect) by the charges and interest, capped at the total
outstanding, so the extra amount is genuinely collected and the
portions sum exactly to the net effect.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Charges were dropped entirely on days where nothing was due, which
silently discarded e.g. an insufficient-funds charge attached to a
failed payment retry on a non-schedule day. Only charges relating to
non-payment of an amount due (late-payment charges) are now suppressed
on such days.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…principal on day-0 schedules

The rounding residual is now absorbed by regenerating the schedule with
the principal balance floored at zero: the final payment closes the
balance exactly, payments can never go negative, and any overpayment
that exceeds the final payment shortens the schedule by dropping the
trailing zeroed payments. The seed item produced by Array.scan is no
longer matched by the adjustment, which previously doubled the
principal total when the only payment fell on day 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… no payment days

The add-on interest method previously crashed with an uncontrolled
'The input array was empty' exception and the actuarial method returned
a degenerate one-item schedule. The message points out that schedules
whose unit-period config starts before the loan start date are silently
ignored, as this is an easy one-day-off mistake to make.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rest rates

Three compounding fixes:
- the rough interest estimate for the actuarial method now respects the
  total interest cap, so the initial guess is no longer far above the
  true payment value when the cap binds
- solveBisection no longer assumes the root lies within a fixed
  [0.75, 1.25] multiple of the initial guess: it now checks whether the
  root is bracketed and expands the bounds geometrically (bounded to
  [0, 4x the guess]) before bisecting
- when no payment value can satisfy even the maximum solver tolerance
  (at high rates a one-cent payment change can move the final balance
  by more than the tolerance), the nearest viable payment value around
  the solver's best approximation is used instead, and any residual is
  absorbed by the final-payment adjustment

Solver failures now also raise descriptive errors including the
solution state and key parameters instead of a bare 'Unable to
calculate basic schedule'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… fixes

Covers: level-payment solving under a binding total interest cap for
5-60 monthly payments, very high daily rates for 4-24 monthly payments,
descriptive failure on schedule configs yielding no payment days,
non-negative final payments with schedule shortening on overpayment,
and correct principal total for a single payment on day 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	tests/PaymentScheduleTests.fs
…validation

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Thorium

Thorium commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

If you want, I can split these into multiple smaller PRs, but there are already quite many PRs in this repo, so maybe better to keep like this.

This branch has not been deployed

No deployments
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