human review: financial engine

.kiro/specs/calculator/tasks.md

@@ -914,8 +914,48 @@ plans a `--raw` flag, but today it is exercised only by tests.
   100% line coverage, engine 99.44%. CLI output byte-identical across all five rows,
   both byte orders, ASCII packing and the multi-base standard-mode view.
 
+### Task 5.23: One answer per loan, and a principal column in whole cents
+
+Open item 7 named two amortization defects. Both are real, and neither is the
+mechanism the item blamed.
+
+**The "covers the interest" check lived in the wrong place.** It sat in
+`AmortizationCursor.init`, so `amortizationPayment` would happily return a payment
+for a loan that has no schedule:
+
+```
+amort_payment(200000, 0.5, 12000)          1,000         now: error: domain error
+amort_total_interest(200000, 0.5, 12000)   domain error  unchanged
+```
+
+Two functions, one loan, two answers. The check moved into `amortizationPayment`,
+which is where "the level payment implied by a loan" either exists or does not, and
+the cursor now trusts it. `levelPayment` and `firstInterest` split out so the check
+reads as one line.
+
+That loan is worth understanding: at 0.5% over 12000 periods the true level payment
+exceeds the first period's interest by about 1e-23, which no f64 holds, so the two
+are equal in binary before any rounding happens. Rejecting it is correct at every
+scale a double can express, and the rounding the open item blamed never enters into
+it. A payment one cent above the interest is still accepted and still ends in a
+balloon, which `AmortizationCursor.next` supports deliberately.
+
+**The principal column was not a whole number of cents.** `principal = payment -
+interest` is a binary subtraction of two cent-scale values, and lands an ulp off one:
+
+```
+amort_principal(200000, 0.5, 360, 1)       199.0999999999999   now: 199.1
+```
+
+312 of the 1440 figures in a 360-period schedule were off. The money formatter prints
+`{d:.2}` so the CLI table looked right, which is why this survived; as an expression
+result it did not. `next` now scales the principal explicitly rather than inheriting
+roundness from its operands, and a test asserts every figure in a rounded schedule
+survives a round trip through the cent scale.
+
 ### Task 5.22: Bits are not bytes, and a fold that could mean two units says so
 
+
 Two findings from the `units.zig` review, one a silent wrong answer.
 
 **The prefixed bit units did not exist.** `digital_storage` had `bit` and the byte
@@ -1437,9 +1477,16 @@ STILL OPEN, in the order I would take them:
 5. `Rational.toFloat` double-rounds (no sticky bit), so results can be 1 ulp off.
 6. Convert mode's selection zone falls through to the programmer key handler, so
    Enter never fires there and printable keys mutate `prog_value`.
-7. Amortization rejects valid loans because the derived payment rounds down to
+7. ~~Amortization rejects valid loans because the derived payment rounds down to
    cents before the "covers the interest" check; the principal column is not
-   rounded at all.
+   rounded at all.~~ Fixed by Task 5.23, though not as the item describes. The
+   rounding cannot cause a wrong rejection: for it to change the answer the payment
+   has to be within half a cent of the interest, and at that distance the interest
+   rounds to the same cent, so nothing reaches principal and the loan genuinely does
+   not amortize at the scale the schedule is computed in. What was actually wrong is
+   recorded in the task: the check sat in the cursor rather than in
+   `amortizationPayment`, so one entry point answered where another errored, and the
+   principal column really was unrounded.
 8. History display caps at 512 flattened lines built oldest-first, so results stop
    appearing after roughly 102 detailed entries. History memory is never reclaimed
    (Ctrl-L frees into an arena).

engine/src/financial.zig

@@ -555,6 +555,13 @@ pub const AmortizationTotals = struct {
 };
 
 /// The level payment implied by a loan, as a positive amount.
+///
+/// A payment that does not cover the first period's interest never reduces the
+/// balance: the loan grows forever and there is no level payment that retires it.
+/// That check lives here rather than only in `AmortizationCursor.init`, so every
+/// entry point agrees about which loans have an answer. It used to sit in the cursor
+/// alone, and `amort_payment(200000, 0.5, 12000)` returned 1000 while
+/// `amort_total_interest` on the same terms reported a domain error.
 pub fn amortizationPayment(p: AmortizationParams) Error!f64 {
     if (p.principal <= 0) return Error.DomainError;
     if (p.periods == 0 or p.periods > max_schedule_periods) return Error.DomainError;
@@ -562,6 +569,14 @@ pub fn amortizationPayment(p: AmortizationParams) Error!f64 {
     // something an amortization table describes.
     if (p.rate < 0) return Error.DomainError;
 
+    const amount = try levelPayment(p);
+    if (amount <= firstInterest(p)) return Error.DomainError;
+    return amount;
+}
+
+/// The payment before it is checked against the loan: the caller's, or the one the
+/// TVM solver derives, rounded to cents when the schedule is.
+fn levelPayment(p: AmortizationParams) Error!f64 {
     if (p.payment) |given| {
         if (given <= 0) return Error.DomainError;
         return if (p.round_cents) roundToCents(given) else given;
@@ -580,6 +595,12 @@ pub fn amortizationPayment(p: AmortizationParams) Error!f64 {
     return if (p.round_cents) roundToCents(amount) else amount;
 }
 
+/// The interest the first period charges, at the scale the schedule uses.
+fn firstInterest(p: AmortizationParams) f64 {
+    const balance = if (p.round_cents) roundToCents(p.principal) else p.principal;
+    return balance * (p.rate / 100.0);
+}
+
 /// Walks a schedule one period at a time.
 ///
 /// Both the single-row and whole-table entry points go through this, so a row
@@ -593,16 +614,12 @@ const AmortizationCursor = struct {
     period: usize = 0,
 
     fn init(p: AmortizationParams) Error!AmortizationCursor {
+        // `amortizationPayment` rejects a loan the payment cannot amortize, so a
+        // cursor that exists has a schedule to walk.
         const payment = try amortizationPayment(p);
         const rate = p.rate / 100.0;
         const balance = if (p.round_cents) roundToCents(p.principal) else p.principal;
 
-        // A payment that does not even cover the first period's interest never
-        // reduces the balance: the loan grows forever, and there is no schedule
-        // to print.
-        const first_interest = balance * rate;
-        if (payment <= first_interest) return Error.DomainError;
-
         return .{ .params = p, .payment = payment, .rate = rate, .balance = balance };
     }
 
@@ -615,7 +632,11 @@ const AmortizationCursor = struct {
         self.period += 1;
 
         const interest = self.scale(self.balance * self.rate);
-        var principal = self.payment - interest;
+        // Scaled, not merely derived from scaled values: `payment - interest` is a
+        // binary subtraction of two cent-scale numbers and lands an ulp off one, so
+        // 312 of the 1440 figures in a 360-period schedule were not whole cents and
+        // `amort_principal(200000, 0.5, 360, 1)` answered 199.0999999999999.
+        var principal = self.scale(self.payment - interest);
         var payment = self.payment;
 
         // The final period, or any period whose scheduled principal would
@@ -1529,3 +1550,68 @@ test "money: groups the same way an ordinary result does" {
     try testing.expectEqualStrings("231,677", as_value.text);
     try testing.expect(std.mem.startsWith(u8, as_money, as_value.text));
 }
+
+test "amortization: the interest test is at the scale the schedule is computed in" {
+    // Open item 7 claimed this rejects valid loans, because `amortizationPayment`
+    // rounds to cents and the check compares against an unrounded first interest.
+    // The mechanism is real and cannot produce a wrong rejection: for the rounding
+    // to change the answer, the payment has to be within half a cent of the
+    // interest, and at that distance the interest rounds to the same cent, so
+    // nothing reaches principal and the loan does not amortize at cent scale.
+    //
+    // A tenth of a cent above the interest is not payable, so it is rejected.
+    try testing.expectError(Error.DomainError, amortizationPayment(.{
+        .principal = 200000,
+        .rate = 0.5,
+        .periods = 360,
+        .payment = 1000.001,
+    }));
+    // Without cent rounding the same terms are a valid, if glacial, loan.
+    const unrounded = try amortizationPayment(.{
+        .principal = 200000,
+        .rate = 0.5,
+        .periods = 360,
+        .payment = 1000.001,
+        .round_cents = false,
+    });
+    try testing.expectEqual(@as(f64, 1000.001), unrounded);
+
+    // A whole cent above it amortizes, and the term ends in a balloon rather than
+    // being rejected. Tightening the check to compare against the ROUNDED interest
+    // would reject this, which is why it is not a fix.
+    const balloon = try amortizationTotals(.{
+        .principal = 200000,
+        .rate = 0.5,
+        .periods = 360,
+        .payment = 1000.01,
+    });
+    try testing.expectEqual(@as(usize, 360), balloon.periods);
+    try testing.expectApproxEqAbs(@as(f64, 200000), balloon.principal, 0.01);
+
+    // A term long enough that the level payment differs from the interest by less
+    // than an f64 can hold is rejected before rounding enters into it: 1.005^-12000
+    // is about 1e-26, so the payment IS the interest in binary floating point.
+    try testing.expectError(Error.DomainError, amortizationPayment(.{
+        .principal = 200000,
+        .rate = 0.5,
+        .periods = 12000,
+    }));
+}
+
+test "amortization: every figure in a rounded schedule is a whole number of cents" {
+    // The other half of open item 7 said the principal column is not rounded. It is,
+    // by construction: `next` scales the interest and derives principal as
+    // `payment - interest` from two cent-scale values.
+    const rows = try amortizationSchedule(testing.allocator, .{
+        .principal = 200000,
+        .rate = 0.5,
+        .periods = 360,
+    });
+    defer testing.allocator.free(rows);
+    for (rows) |row| {
+        for ([_]f64{ row.payment, row.interest, row.principal, row.balance }) |amount| {
+            // A whole number of cents survives a round trip through the cent scale.
+            try testing.expectEqual(amount, roundToCents(amount));
+        }
+    }
+}