diff --git a/src/domain/occurrences.test.ts b/src/domain/occurrences.test.ts index 39fe380..68db045 100644 --- a/src/domain/occurrences.test.ts +++ b/src/domain/occurrences.test.ts @@ -150,9 +150,30 @@ describe("occurrencesOf — confirmation (calendar)", () => { expect(found[0]!.confirmedBy).toBe("pY"); expect(found[1]!.confirmedBy).toBeNull(); }); + + test("REVIEW ROUND 2 / I2: a calendar-cost purchase with no occurrenceDate throws, naming the purchase", () => { + // Reachable from legal-looking imported data, not just a programming + // error: the JSON importer currently accepts a purchase referencing a + // recurring cost with no occurrenceDate, so a file built that way loads + // clean and throws on the very next fold. Pinned here so the throw's + // behaviour (and its message naming the purchase, not a bare "undefined") + // doesn't drift silently. + const rent = cost({ id: "rent", anchoring: "calendar" }); + const noSlot: Purchase = { + id: "bad1", + date: "2026-01-05", + description: "Rent", + total: { amount: 8000, currency: "DKK" }, + splitMode: "percent", + splits: [{ postId: "housing", value: 100, absorbsRemainder: true }], + schedule: null, + source: { recurringId: "rent" }, // no occurrenceDate + }; + expect(() => occurrencesOf(rent, [noSlot], "2026-02")).toThrow(/bad1/); + }); }); -describe("occurrencesOf — termination", () => { +describe("occurrencesOf — termination (grid walk)", () => { // CONTROLLER RULING: the round-1 version of this block also had "a // confirmation dated absurdly early throws rather than walking backwards", // which modelled the OLD lastCharge slot-rebasing mechanism (a confirmation @@ -163,8 +184,15 @@ describe("occurrencesOf — termination", () => { // before startDate cannot pull the series backwards" below) — so the throw // that test asserted is no longer reachable by the scenario it built. // Deleted rather than adapted, because there is no reachable input left that - // exercises the same path; the anchored walk's own termination throw is - // still covered by the two tests kept below, which only need n < 1. + // exercises the same path. + // + // CORRECTION (round 2 review): the two tests below both build their cost + // with `cost({...})`, whose fixture default is `anchoring: "calendar"` — so + // they only ever exercise `gridOccurrences`'s termination throw, not + // `anchoredOccurrences`'s. A prior version of this comment claimed the + // anchored throw was "still covered by the two tests kept below"; that was + // false. The anchored walk's own termination throw has its own test, in + // "the anchored walk (lastCharge)" below. test("a recurrence that cannot advance throws rather than hanging", () => { const broken = cost({ recurrence: { kind: "everyNDays", n: 0 }, startDate: "2026-01-01" }); expect(() => occurrencesOf(broken, none, "2026-02")).toThrow(/did not advance/); @@ -309,6 +337,28 @@ describe("the anchored walk (lastCharge)", () => { expect(dates).toEqual(["2026-10-12", "2026-11-11"]); }); + test("REVIEW ROUND 2 / I1: a broken recurrence throws through the ANCHORED walk too, not just the grid one", () => { + // Proven missing by mutation: replacing the anchored walk's termination + // throw with a `break` left every existing test passing, because the two + // tests in "occurrencesOf — termination (grid walk)" both default to + // `anchoring: "calendar"` via the `cost()` fixture and so only exercise + // `gridOccurrences`. `phone()` here defaults to `anchoring: "lastCharge"`, + // so this one actually walks `anchoredOccurrences`. + const broken = phone({ recurrence: { kind: "everyNDays", n: 0 } }); + expect(() => occurrencesOf(broken, [], "2026-11")).toThrow(/did not advance/); + }); + + test("REVIEW ROUND 2 / C1: a month-only startDate throws a cost-named error, not a bare date-parsing one", () => { + // No charges, so the walk's starting cursor is `cost.startDate` directly + // — "2026-09", month-only. Before this fix, that reached `stepFrom` -> + // `addMonthsToDate` and threw "Not a day-granular IsoDate: 2026-09", + // naming no cost, three frames below foldBalances. Now `anchoredOccurrences` + // catches it first and names the bill. + const untimed = phone({ startDate: "2026-09" }); + expect(() => occurrencesOf(untimed, [], "2026-11")).toThrow(/Phone plan/); + expect(() => occurrencesOf(untimed, [], "2026-11")).not.toThrow(/day-granular IsoDate/); + }); + test("unconfirmed across many months, a day-granular anchored cost just keeps stepping", () => { // CONTROLLER RULING: replaces round-1's "unconfirmed, it behaves exactly // like a calendar series" (old describe "lastCharge anchoring: the phone diff --git a/src/domain/occurrences.ts b/src/domain/occurrences.ts index bd51699..80c8e9e 100644 --- a/src/domain/occurrences.ts +++ b/src/domain/occurrences.ts @@ -42,6 +42,9 @@ function isBefore(a: IsoDate, b: IsoDate): boolean { return a < b; } +/** "YYYY-MM-DD" vs "YYYY-MM" — whether a date carries a day at all. */ +const DAY_GRANULAR_DATE = /^\d{4}-\d{2}-\d{2}$/; + /** Moves forward to the next occurrence of `weekday`, or stays put if already on it. */ function snapForward(date: IsoDate, weekday: number): IsoDate { const delta = (weekday - weekdayOf(date) + 7) % 7; @@ -58,13 +61,16 @@ function snapForward(date: IsoDate, weekday: number): IsoDate { * step is day-precise and clamps (`addMonthsToDate`). The other two kinds are * day-granular either way and ignore the argument. * - * The anchored walk (`anchoredOccurrences`) only ever calls this with a - * day-granular `date`: its cursor starts at `cost.startDate` or at a real - * charge's date, and an anchored `everyNMonths` cost is required to carry a - * day-granular `startDate` for exactly this reason. If a month-only date ever - * reaches this branch, `addMonthsToDate` throwing is CORRECT — something - * upstream produced a cursor this function was never meant to receive, and a - * visible error beats silently treating "2026-09" as "2026-09-01". + * `anchoredOccurrences` checks its starting cursor is day-granular before + * ever calling this for an `everyNMonths` cost, and throws an error naming + * the cost if it is not. That check exists BECAUSE nothing currently + * guarantees a day-granular `startDate` on an anchored `everyNMonths` cost — + * neither write boundary rejects a month-only one, and a charge's own date + * can be month-only too (`Purchase.date`'s documented shape). Without that + * check, a month-only date reaching this branch would still throw, but from + * `addMonthsToDate` with a bare parsing message that names no cost — a + * failure the owner cannot act on, several frames below `foldBalances`, that + * would take every money route down with it. * * Adding a recurrence kind means adding a case here and a variant to * `Recurrence`. Nothing else in the app switches on the kind. @@ -96,14 +102,17 @@ export function stepFrom( * * Calendar-only. A `lastCharge` cost has no slots (`Purchase.source`'s doc in * types.ts) — its charges are walked by `chargesFor`/`anchoredOccurrences` - * off `Purchase.date` directly, and never reach this function. A purchase - * that DOES reach here (its `recurringId` matches) but carries no - * `occurrenceDate` is therefore not a data problem to skip past quietly: it - * means some caller attached a `lastCharge`-shaped purchase (or one with a - * dangling/mismatched source) to a cost this function is walking as - * `calendar`, which is a programming error upstream. Throwing says so - * directly instead of the old scaffolding `continue`, which would have - * silently dropped the confirmation and left the slot looking unpaid. + * off `Purchase.date` directly, and never reach this function BY DESIGN. + * + * A purchase that DOES reach here (its `recurringId` names a `calendar` cost) + * but carries no `occurrenceDate` is not only a hypothetical programming + * error: the JSON importer currently accepts a purchase missing one, and + * import replaces the whole dataset, so a file built that way loads clean and + * throws on the very next fold — the app goes down immediately, from data the + * import boundary let in. Throwing here, naming the purchase, is still better + * than the old scaffolding `continue`, which would have silently dropped the + * confirmation and left the slot looking unpaid; closing the import boundary + * itself is a separate piece of work. */ export function confirmationsFor( purchases: readonly Purchase[], @@ -183,6 +192,23 @@ function anchoredOccurrences( // `max(latest, startDate)`: a charge back-dated before the cost existed must // not pull the series behind its own start. const anchor = latest === null ? null : isBefore(latest, cost.startDate) ? cost.startDate : latest; + const startingPoint = anchor ?? cost.startDate; + + // `stepFrom`'s `everyNMonths`/`lastCharge` branch hands its date straight + // to `addMonthsToDate`, which requires "YYYY-MM-DD" and would otherwise + // throw a bare parsing error naming no cost at all, several frames below + // `foldBalances` — taking every money route down with a message the owner + // cannot act on. Nothing at either write boundary rejects a month-only + // `startDate` on an anchored `everyNMonths` cost today (and a charge's own + // date can be month-only too, per `Purchase.date`'s documented shape), so + // check here, before the walk starts, and name the cost. + if (cost.recurrence.kind === "everyNMonths" && !DAY_GRANULAR_DATE.test(startingPoint)) { + throw new Error( + `Recurring cost "${cost.name}" is anchored (lastCharge) with a monthly recurrence, so every ` + + `date it steps from must be day-granular ("YYYY-MM-DD") — got "${startingPoint}". Give it a ` + + `specific day: on its startDate if it has no charges yet, or on its latest charge otherwise.`, + ); + } let cursor = anchor === null ? cost.startDate : stepFrom(anchor, cost.recurrence, cost.anchoring); diff --git a/src/store/actions.test.ts b/src/store/actions.test.ts index 4536547..63fd846 100644 --- a/src/store/actions.test.ts +++ b/src/store/actions.test.ts @@ -833,3 +833,56 @@ describe("confirmOccurrence", () => { // always succeeds — because the anchored walk always re-derives its cursor // from whichever charge is latest, never from a `Purchase.date` a caller is // trying to move "behind" a slot that no longer exists. + +// REVIEW ROUND 2 / I3: that claim ("now behaves identically — the edit always +// succeeds") was asserted above but never actually pinned by a test — the +// deleted test #10 ("a date more than one cycle before the slot it claims is +// refused") half-covered this shape and was removed along with it, leaving a +// silent gap. Inverted rather than left out: same fixture the old refused-edit +// test used, but now asserting the edit SUCCEEDS, the fold still folds +// cleanly afterwards, and the series re-anchors to the edited date rather +// than the original confirmation date. +describe("editing a charge's date backwards re-anchors instead of throwing", () => { + test("moving a confirmed charge's date more than one cycle earlier re-anchors the series to it", () => { + const data = createSeedDataset("2026-08"); + data.posts[0]!.id = "housing"; + const car = actions.addRecurringCost(data, { + name: "Car insurance", + archived: false, + amount: { amount: 900, currency: "DKK" }, + startDate: "2026-08-14", + recurrence: { kind: "everyNDays", n: 30 }, + anchoring: "lastCharge", + splitMode: "percent", + splits: [{ postId: "housing", value: 100, absorbsRemainder: true }], + }); + + const purchase = actions.confirmOccurrence(data, car.id, "2026-09-13", { + date: "2026-09-13", + }); + expect(() => occurrencesByMonth(data, "2026-12")).not.toThrow(); + + // Under the old guard, an edit like this ("more than one cycle before the + // slot it claims") was refused. Now it just succeeds. (2026-08-20, not + // something before the cost's own 2026-08-14 startDate — an edit that far + // back would hit the DELIBERATE "can't pull the series before startDate" + // clamp tested in occurrences.test.ts, which is a different behaviour + // than the one this test is pinning.) + actions.updatePurchase(data, purchase.id, { date: "2026-08-20" }); + expect(data.purchases.find((p) => p.id === purchase.id)!.date).toBe("2026-08-20"); + + // The fold still works... + expect(() => occurrencesByMonth(data, "2026-12")).not.toThrow(); + + // ...and the series re-anchors to the EDITED date: the next projected + // occurrence is one cycle (30 days) after 2026-08-20, not after the + // original 2026-09-13. + const byMonth = occurrencesByMonth(data, "2026-12"); + const dates = [...byMonth.values()] + .flat() + .filter((o) => o.recurringId === car.id) + .map((o) => o.date) + .sort(); + expect(dates[0]).toBe("2026-09-19"); // 2026-08-20 + 30 days + }); +}); diff --git a/src/ui/components/ExpectedBand.test.ts b/src/ui/components/ExpectedBand.test.ts index 8b4f026..ce25d06 100644 --- a/src/ui/components/ExpectedBand.test.ts +++ b/src/ui/components/ExpectedBand.test.ts @@ -298,6 +298,33 @@ describe("expectedGroups — X3 (a cost with an outstanding slot never also show // throws — is still true, and now trivially so: nothing in the anchored walk // can throw from a confirmation's date any more. describe("expectedGroups — coming-up confirmations never break the fold (formerly X1)", () => { + test("REVIEW ROUND 2 (Minor): a day-granular anchored cost's future slot IS now offered once this month is settled — X1's suppression is gone, not replaced", () => { + // This is deleted test #8's scenario ("stays suppressed even once X3 no + // longer applies") — still perfectly reachable, and now completely + // untested. Pinning what actually happens today rather than leaving the + // gap silent: September's slot is settled on time, so X3 has nothing left + // to suppress, and there is no anchored-model X1 replacement, so the + // October row (39 days from `today`, more than one 30-day cycle) IS + // offered. If a future task reintroduces some suppression here, this test + // will fail loudly and force that change to be deliberate. + const data = baseDataset(); + data.recurring = [ + cost({ + id: "car", + name: "Car insurance", + amount: { amount: 349, currency: "DKK" }, + startDate: "2026-08-14", + recurrence: { kind: "everyNDays", n: 30 }, + anchoring: "lastCharge", + }), + ]; + confirmOccurrence(data, "car", "2026-09-13", { date: "2026-09-13" }); + + const groups = expectedGroups(data, "2026-09", "2026-09-04"); + expect(groups.pending).toEqual([]); + expect(groups.comingUp.map((o) => o.date)).toEqual(["2026-10-13"]); + }); + test("regression: whatever expectedGroups offers as 'coming up', confirming it all today and folding never throws (demo Car insurance shape)", () => { const data = baseDataset(); data.recurring = [