From d542a6bdab53efc4fdf7999d5e6a5afd439cb276 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Sun, 6 Sep 2026 18:31:40 +0200 Subject: [PATCH] fix(domain): the anchored walk is day-precise under every recurrence kind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The precheck in `anchoredOccurrences` covered `everyNMonths` only, so a month-only starting point on an `everyNDays` or `everyNWeeks` anchored cost fell straight through to `addDays` and threw a bare "Not a day-granular IsoDate", naming no cost, several frames below `foldBalances` — the exact failure the precheck exists to prevent, for two thirds of the costs that can have it. The walk is day-precise under all three kinds, so the check is too. The rule about WHICH dates need a day was written out separately in four places (both write boundaries and both controls that offer the pairings). It now lives once, in `granularity.ts`, alongside the second, different rule the next commits need: an anchored cost has no slots, so its latest CHARGE's own `Purchase.date` is the anchor the series steps from, whatever the kind. Three comments corrected to describe the code that exists: - `stepFrom` and `anchoredOccurrences` both claimed neither write boundary rejects a month-only `startDate`. Both do, and have since this model landed. Pointing the reader at the closed hole is why the open one — the CHARGE's own date — survived four reviews. - `Anchoring`'s doc still said confirming the occurrence early is the whole mechanism. An anchored cost has no occurrence to confirm; recording a charge is the mechanism. - `Occurrence.date` described slot identity as universal (it is calendar-only), and `confirmedBy` did not say it is unconditionally null on the anchored path. Co-Authored-By: Claude Opus 5 (1M context) --- src/domain/granularity.test.ts | 71 ++++++++++++++++++++++++++++++++++ src/domain/granularity.ts | 51 ++++++++++++++++++++++++ src/domain/index.ts | 1 + src/domain/occurrences.test.ts | 39 +++++++++++++++++++ src/domain/occurrences.ts | 68 ++++++++++++++++++-------------- src/domain/types.ts | 18 +++++++-- 6 files changed, 216 insertions(+), 32 deletions(-) create mode 100644 src/domain/granularity.test.ts create mode 100644 src/domain/granularity.ts diff --git a/src/domain/granularity.test.ts b/src/domain/granularity.test.ts new file mode 100644 index 0000000..a879e6a --- /dev/null +++ b/src/domain/granularity.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, test } from "bun:test"; +import { + chargeDateNeedsDay, + isDayGranularDate, + startDateNeedsDay, +} from "./granularity.ts"; + +describe("isDayGranularDate", () => { + test("a day-granular date carries a day; a month-only one does not", () => { + expect(isDayGranularDate("2026-09-17")).toBe(true); + expect(isDayGranularDate("2026-09")).toBe(false); + }); + + test("an empty or malformed value is not day-granular", () => { + expect(isDayGranularDate("")).toBe(false); + expect(isDayGranularDate("not-a-date")).toBe(false); + expect(isDayGranularDate("2026-9-1")).toBe(false); + }); + + test("shape only — a calendar-impossible day still reads as day-granular", () => { + // Deliberate: whether a date is REAL is a separate question, answered by + // `toDayOrdinal` at the boundaries that need it. + expect(isDayGranularDate("2026-09-31")).toBe(true); + expect(isDayGranularDate("2026-02-30")).toBe(true); + }); +}); + +/** + * The one statement of a rule that used to be written out in four places — + * both write boundaries and both controls that offer these pairings. Four + * copies is how a fifth place ends up without one, which is exactly what + * happened to the CHARGE side of the same rule below. + */ +describe("startDateNeedsDay", () => { + test("everyNMonths under calendar is the ONLY pairing that tolerates a month", () => { + expect(startDateNeedsDay("everyNMonths", "calendar")).toBe(false); + }); + + test("everyNMonths under lastCharge needs a day — it steps from a real charge date", () => { + expect(startDateNeedsDay("everyNMonths", "lastCharge")).toBe(true); + }); + + test("everyNDays and everyNWeeks need a day under either anchoring — they reach addDays", () => { + expect(startDateNeedsDay("everyNDays", "calendar")).toBe(true); + expect(startDateNeedsDay("everyNDays", "lastCharge")).toBe(true); + expect(startDateNeedsDay("everyNWeeks", "calendar")).toBe(true); + expect(startDateNeedsDay("everyNWeeks", "lastCharge")).toBe(true); + }); +}); + +/** + * A DIFFERENT rule from the one above, because the two dates are read + * differently: a calendar charge names the slot it settles and its own date is + * never stepped from, while an anchored cost has no slots at all and its + * latest charge's date IS the anchor. + */ +describe("chargeDateNeedsDay", () => { + test("an anchored cost's charge needs a day; a calendar cost's does not", () => { + expect(chargeDateNeedsDay("lastCharge")).toBe(true); + expect(chargeDateNeedsDay("calendar")).toBe(false); + }); + + test("it does not depend on the recurrence kind, unlike the startDate rule", () => { + // Every kind is day-precise under `lastCharge`: everyNMonths reaches + // addMonthsToDate, the other two reach addDays. So there is nothing for a + // kind to change here — and `startDateNeedsDay` agrees on that side. + for (const kind of ["everyNMonths", "everyNDays", "everyNWeeks"] as const) { + expect(startDateNeedsDay(kind, "lastCharge")).toBe(true); + } + }); +}); diff --git a/src/domain/granularity.ts b/src/domain/granularity.ts new file mode 100644 index 0000000..6f0bca5 --- /dev/null +++ b/src/domain/granularity.ts @@ -0,0 +1,51 @@ +import type { Anchoring, IsoDate, Recurrence } from "./types.ts"; + +/** + * "YYYY-MM-DD" vs "YYYY-MM" — whether a date carries a day at all. + * + * Shape only, and deliberately: "2026-09-31" is day-GRANULAR and calendar- + * impossible, and the two are separate questions. A boundary that needs the + * second one routes the value through `toDayOrdinal`/`monthOf`, which already + * throw on exactly that. + */ +const DAY_GRANULAR_DATE = /^\d{4}-\d{2}-\d{2}$/; + +/** Whether `date` names a specific day, rather than a whole month. */ +export function isDayGranularDate(date: IsoDate): boolean { + return DAY_GRANULAR_DATE.test(date); +} + +/** + * Whether a recurring cost's `startDate` must name a specific day, given the + * schedule it describes. The ONE statement of this rule: both write + * boundaries (`store/actions.ts`, `export/json.ts`) and both controls that + * offer these pairings (`ui/recurringForm.ts`, the Settings recurring table) + * call this rather than restating it, because four copies of a rule is how a + * fifth place ends up without one. + * + * `everyNMonths` under `calendar` anchoring is the single exception: that grid + * walks its cursor through `monthOf`, so a month-only date is exactly right — + * rent occurs IN a month. Every other pairing walks day-precisely: + * `everyNDays` and `everyNWeeks` hand the date straight to `addDays` + * regardless of anchoring, and `everyNMonths` under `lastCharge` steps through + * `addMonthsToDate` because the next due date is measured from a real charge. + */ +export function startDateNeedsDay(kind: Recurrence["kind"], anchoring: Anchoring): boolean { + return kind !== "everyNMonths" || anchoring === "lastCharge"; +} + +/** + * Whether a CHARGE's own `Purchase.date` must name a specific day, given the + * anchoring of the cost it names. + * + * A separate rule from `startDateNeedsDay`, because the two dates are read + * differently. A `calendar` cost's charge is only ever placed in a month (its + * slot lives in `source.occurrenceDate`), so a month-only purchase date is + * fine there whatever the recurrence kind. A `lastCharge` cost has no slots at + * all: `anchoredOccurrences` takes the LATEST charge's own date as its anchor + * and steps forward from it, so that date reaches `addDays`/`addMonthsToDate` + * under every recurrence kind and must carry a day. + */ +export function chargeDateNeedsDay(anchoring: Anchoring): boolean { + return anchoring === "lastCharge"; +} diff --git a/src/domain/index.ts b/src/domain/index.ts index ed838dc..d0daf23 100644 --- a/src/domain/index.ts +++ b/src/domain/index.ts @@ -4,6 +4,7 @@ export * from "./types.ts"; export * from "./money.ts"; export * from "./months.ts"; export * from "./days.ts"; +export * from "./granularity.ts"; export * from "./fx.ts"; export * from "./allocation.ts"; export * from "./charges.ts"; diff --git a/src/domain/occurrences.test.ts b/src/domain/occurrences.test.ts index 68db045..d0c739c 100644 --- a/src/domain/occurrences.test.ts +++ b/src/domain/occurrences.test.ts @@ -359,6 +359,45 @@ describe("the anchored walk (lastCharge)", () => { expect(() => occurrencesOf(untimed, [], "2026-11")).not.toThrow(/day-granular IsoDate/); }); + // The precheck above covered `everyNMonths` only, so the other two kinds + // fell straight through to `addDays` and threw a BARE "Not a day-granular + // IsoDate", naming no cost — the failure the precheck exists to prevent, + // for two thirds of the anchored costs that can have it. This walk is + // day-precise under every kind, so the check is too. + test("a month-only startDate names the cost under everyNDays as well", () => { + const daily = phone({ recurrence: { kind: "everyNDays", n: 30 }, startDate: "2026-09" }); + expect(() => occurrencesOf(daily, [], "2026-11")).toThrow(/Phone plan/); + expect(() => occurrencesOf(daily, [], "2026-11")).not.toThrow(/day-granular IsoDate/); + }); + + test("a month-only startDate names the cost under everyNWeeks as well", () => { + const weekly = phone({ + recurrence: { kind: "everyNWeeks", n: 2, weekday: 4 }, + startDate: "2026-09", + }); + expect(() => occurrencesOf(weekly, [], "2026-11")).toThrow(/Phone plan/); + expect(() => occurrencesOf(weekly, [], "2026-11")).not.toThrow(/day-granular IsoDate/); + }); + + // Not just `startDate`: the anchor is the LATEST CHARGE's own date once + // there is one, and `Purchase.date`'s documented shape allows "YYYY-MM". + // Both write boundaries refuse that now, but the walk still has to name the + // cost rather than throw a bare parse error for a dataset they never saw. + test("a month-only CHARGE date names the cost too, under every kind", () => { + for (const recurrence of [ + { kind: "everyNMonths", n: 1 }, + { kind: "everyNDays", n: 30 }, + { kind: "everyNWeeks", n: 2, weekday: 4 }, + ] as const) { + // startDate before the charge, so `max(latest, startDate)` picks the + // charge's own month-only date as the anchor. + const cost = phone({ recurrence, startDate: "2026-08-04" }); + const charges = [charge("p1", "2026-09")]; + expect(() => occurrencesOf(cost, charges, "2026-11")).toThrow(/Phone plan/); + expect(() => occurrencesOf(cost, charges, "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 80c8e9e..d47339c 100644 --- a/src/domain/occurrences.ts +++ b/src/domain/occurrences.ts @@ -1,4 +1,5 @@ import { addDays, addMonthsToDate, weekdayOf } from "./days.ts"; +import { isDayGranularDate } from "./granularity.ts"; import { addMonths, compareMonths, monthOf } from "./months.ts"; import type { Anchoring, @@ -22,13 +23,24 @@ import type { export interface Occurrence { recurringId: RecurringCostId; /** - * The slot this walk generated. IDENTITY: a confirmation claims a slot by - * this date, which is not necessarily the date it was paid. + * When this projected charge falls. + * + * For a `calendar` cost this date is also an IDENTITY: it names a slot on a + * fixed grid, and a charge claims that slot through + * `Purchase.source.occurrenceDate`, which is not necessarily the date it was + * paid. A `lastCharge` cost has no grid and no slots, so nothing ever refers + * back to this date — it is a projection and nothing more. */ date: IsoDate; /** Expected amount, in the cost's own currency. */ amount: Money; - /** The purchase that confirmed this slot, or null while it is still expected. */ + /** + * The purchase that confirmed this slot, or null while it is still expected. + * + * Calendar-only in practice: the anchored walk sets this to null + * unconditionally, because a charge that happened is a `Purchase` counted as + * an actual, never an occurrence it could confirm. + */ confirmedBy: PurchaseId | null; } @@ -42,9 +54,6 @@ 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; @@ -61,16 +70,17 @@ 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. * - * `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. + * Under `lastCharge` every kind here is day-precise, so `anchoredOccurrences` + * checks its starting cursor is day-granular before ever calling this, and + * throws an error naming the cost if it is not. Both write boundaries reject a + * month-only `startDate` on an anchored cost and a month-only date on a charge + * naming one (`startDateNeedsDay`/`chargeDateNeedsDay` in + * `domain/granularity.ts`, applied in `store/actions.ts` and `export/json.ts`), + * so that check is defensive rather than load-bearing — but it is what stands + * between a dataset those boundaries never saw (a hand-edited store, a bug in + * a later migration step) and a bare `addDays`/`addMonthsToDate` parsing + * message naming no cost, 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. @@ -194,19 +204,21 @@ function anchoredOccurrences( 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)) { + // EVERY recurrence kind, not just `everyNMonths`: this walk is day-precise + // throughout. `everyNMonths` reaches `addMonthsToDate` and the other two + // reach `addDays`, and all three require "YYYY-MM-DD" — so a month-only + // starting point used to 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. Both write boundaries now reject the two + // ways such a date could be stored (a month-only `startDate` on an anchored + // cost, and a month-only date on a charge naming one), so this is the + // backstop for a dataset they never saw; check before the walk starts, and + // name the cost. + if (!isDayGranularDate(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.`, + `Recurring cost "${cost.name}" is anchored (lastCharge), so every date its series 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.`, ); } diff --git a/src/domain/types.ts b/src/domain/types.ts index 9dbf687..7e26c5d 100755 --- a/src/domain/types.ts +++ b/src/domain/types.ts @@ -130,7 +130,10 @@ export type Recurrence = * `lastCharge` — from the date the previous charge ACTUALLY happened. This is * the phone bill: hitting the data cap makes a charge happen early, and the * next 30 days run from there. The domain never learns what a data cap is; - * confirming the occurrence early is the whole mechanism. + * RECORDING A CHARGE is the whole mechanism. There is no occurrence to + * confirm on this side — an anchored cost has no slots, so every occurrence it + * projects is unconfirmed by definition and a charge is an ordinary + * `Purchase`, dated when it happened, whose own date anchors what comes next. */ export type Anchoring = "calendar" | "lastCharge"; @@ -158,9 +161,9 @@ export interface RecurringCost { * `calendar` anchoring, where "YYYY-MM" is also allowed — that grid is * month-granular, but every other combination needs a real day: `everyNDays` * and `everyNWeeks` hand this straight to `addDays`, and `everyNMonths` - * under `lastCharge` steps day-precisely from a real charge date. Both write - * boundaries (`requireStartDateGranularity` in `store/actions.ts`, the - * mirrored check in `export/json.ts`) enforce this pairing. + * under `lastCharge` steps day-precisely from a real charge date. The rule + * itself is `startDateNeedsDay` in `domain/granularity.ts`, and both write + * boundaries (`store/actions.ts`, `export/json.ts`) enforce it. */ startDate: IsoDate; /** Occurrences on or after this date are not projected. This is cancellation. */ @@ -174,6 +177,13 @@ export interface RecurringCost { export interface Purchase { id: PurchaseId; + /** + * When it was bought. Month-only is normal — the app is about monthly + * spending — EXCEPT on a charge for a `lastCharge` recurring cost, where + * this date is what the next due date is measured from and so must name a + * day. That is `chargeDateNeedsDay` in `domain/granularity.ts`, enforced at + * both write boundaries alongside the `source` pairing below. + */ date: IsoDate; description: string; /** Optional longer context, alongside the short `description` label. */