From db5bffbdcf6969f194d65fe56b98cfeabefbed91 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 25 Apr 2026 14:55:20 -0400 Subject: [PATCH] feat(balance): add priced-kind validation to service + tests - Export validateLineKindInvariants helper for both 'simple' and 'priced' account kinds; surfaces typed BalanceServiceError codes. - Extend SnapshotLineInput with optional account_kind / quantity / unit_price (default 'simple' to preserve #146 callers). - upsertSnapshotLines now validates kind invariants ahead of the SQL CHECK and persists priced lines with non-NULL qty / unit_price. - Tolerance constant PRICED_VALUE_TOLERANCE = 0.01 absorbs FP drift. - 14 new unit tests covering simple invariants, priced invariants, tolerance edge cases, and mixed batches. Refs #140 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/services/balance.service.test.ts | 255 +++++++++++++++++++++++++++ src/services/balance.service.ts | 165 ++++++++++++++--- 2 files changed, 394 insertions(+), 26 deletions(-) diff --git a/src/services/balance.service.test.ts b/src/services/balance.service.test.ts index 3e1a361..980415e 100644 --- a/src/services/balance.service.test.ts +++ b/src/services/balance.service.test.ts @@ -23,6 +23,8 @@ import { listLinesBySnapshot, upsertSnapshotLines, getPreviousSnapshot, + validateLineKindInvariants, + PRICED_VALUE_TOLERANCE, BalanceServiceError, } from "./balance.service"; @@ -546,3 +548,256 @@ describe("getPreviousSnapshot", () => { }); }); }); + +// ----------------------------------------------------------------------------- +// Priced-kind validation (Issue #140 / Bilan #2) +// ----------------------------------------------------------------------------- + +describe("validateLineKindInvariants — simple kind", () => { + it("accepts a clean simple line", () => { + expect(() => + validateLineKindInvariants({ account_id: 1, value: 1234.56 }) + ).not.toThrow(); + }); + + it("accepts simple kind with explicit account_kind = simple", () => { + expect(() => + validateLineKindInvariants({ + account_id: 1, + value: 0, + account_kind: "simple", + }) + ).not.toThrow(); + }); + + it("rejects a simple line carrying a quantity", () => { + expect(() => + validateLineKindInvariants({ + account_id: 1, + value: 100, + account_kind: "simple", + quantity: 10, + }) + ).toThrowError(BalanceServiceError); + }); + + it("rejects a simple line carrying a unit_price", () => { + expect(() => + validateLineKindInvariants({ + account_id: 1, + value: 100, + account_kind: "simple", + unit_price: 10, + }) + ).toThrowError(BalanceServiceError); + }); + + it("rejects a non-finite value", () => { + expect(() => + validateLineKindInvariants({ account_id: 1, value: NaN }) + ).toThrowError(BalanceServiceError); + expect(() => + validateLineKindInvariants({ account_id: 1, value: Infinity }) + ).toThrowError(BalanceServiceError); + }); +}); + +describe("validateLineKindInvariants — priced kind", () => { + const baseInput = { + account_id: 7, + account_kind: "priced" as const, + }; + + it("accepts a clean priced line where value === qty * price", () => { + expect(() => + validateLineKindInvariants({ + ...baseInput, + quantity: 10, + unit_price: 25.5, + value: 255, + }) + ).not.toThrow(); + }); + + it("rejects a priced line missing the quantity", () => { + expect(() => + validateLineKindInvariants({ + ...baseInput, + quantity: null, + unit_price: 25.5, + value: 255, + }) + ).toMatchObject; // sanity, real assertion below + expect(() => + validateLineKindInvariants({ + ...baseInput, + quantity: null, + unit_price: 25.5, + value: 255, + }) + ).toThrowError(BalanceServiceError); + try { + validateLineKindInvariants({ + ...baseInput, + quantity: null, + unit_price: 25.5, + value: 255, + }); + } catch (e) { + expect((e as BalanceServiceError).code).toBe( + "snapshot_priced_quantity_required" + ); + } + }); + + it("rejects a priced line missing the unit_price", () => { + try { + validateLineKindInvariants({ + ...baseInput, + quantity: 10, + unit_price: null, + value: 255, + }); + } catch (e) { + expect((e as BalanceServiceError).code).toBe( + "snapshot_priced_unit_price_required" + ); + return; + } + throw new Error("expected throw"); + }); + + it("rejects a priced line where value disagrees with qty × price", () => { + try { + validateLineKindInvariants({ + ...baseInput, + quantity: 10, + unit_price: 25.5, + // off by way more than tolerance — 255.0 expected, 999 saved + value: 999, + }); + } catch (e) { + expect((e as BalanceServiceError).code).toBe( + "snapshot_priced_value_mismatch" + ); + return; + } + throw new Error("expected throw"); + }); + + it("accepts a priced line within the tolerance ε", () => { + // 12.34 × 1.07 = 13.2038 in math, but JS gives 13.2038000000000002. + // The drift is well within ε = 0.01. + const qty = 12.34; + const price = 1.07; + expect(() => + validateLineKindInvariants({ + ...baseInput, + quantity: qty, + unit_price: price, + value: 13.2038, + }) + ).not.toThrow(); + }); + + it("rejects a priced line just outside the tolerance ε", () => { + // expected = 100, threshold ε = 0.01 → 100.011 fails, 100.005 passes. + expect(() => + validateLineKindInvariants({ + ...baseInput, + quantity: 10, + unit_price: 10, + value: 100 + PRICED_VALUE_TOLERANCE * 1.5, + }) + ).toThrowError(BalanceServiceError); + expect(() => + validateLineKindInvariants({ + ...baseInput, + quantity: 10, + unit_price: 10, + value: 100 + PRICED_VALUE_TOLERANCE * 0.5, + }) + ).not.toThrow(); + }); + + it("rejects priced when quantity is non-finite", () => { + expect(() => + validateLineKindInvariants({ + ...baseInput, + quantity: NaN, + unit_price: 10, + value: 100, + }) + ).toThrowError(BalanceServiceError); + }); +}); + +describe("upsertSnapshotLines — priced kind", () => { + it("rejects a priced line where qty × price drifts beyond ε", async () => { + mockSelect.mockResolvedValueOnce([FAKE_SNAPSHOT]); + await expect( + upsertSnapshotLines(5, [ + { + account_id: 7, + account_kind: "priced", + quantity: 10, + unit_price: 25, + value: 999, // wrong on purpose + }, + ]) + ).rejects.toMatchObject({ code: "snapshot_priced_value_mismatch" }); + // No DB mutation when validation fails up-front. + expect(mockExecute).not.toHaveBeenCalled(); + }); + + it("inserts a priced line with quantity + unit_price + value", async () => { + mockSelect.mockResolvedValueOnce([FAKE_SNAPSHOT]); + mockExecute + .mockResolvedValueOnce({ rowsAffected: 1 }) // delete + .mockResolvedValueOnce({ lastInsertId: 100, rowsAffected: 1 }) // insert + .mockResolvedValueOnce({ rowsAffected: 1 }); // bump updated_at + await upsertSnapshotLines(5, [ + { + account_id: 7, + account_kind: "priced", + quantity: 10, + unit_price: 25.5, + value: 255, + }, + ]); + const insertSql = mockExecute.mock.calls[1][0] as string; + expect(insertSql).toContain("INSERT INTO balance_snapshot_lines"); + // Priced inserts use parameter placeholders for qty/price (not literal NULLs) + expect(insertSql).toMatch(/VALUES\s*\(\s*\$1,\s*\$2,\s*\$3,\s*\$4,\s*\$5/); + expect(mockExecute.mock.calls[1][1]).toEqual([5, 7, 10, 25.5, 255]); + }); + + it("supports a mix of simple + priced lines in the same batch", async () => { + mockSelect.mockResolvedValueOnce([FAKE_SNAPSHOT]); + mockExecute + .mockResolvedValueOnce({ rowsAffected: 1 }) // delete + .mockResolvedValueOnce({ lastInsertId: 100, rowsAffected: 1 }) // insert simple + .mockResolvedValueOnce({ lastInsertId: 101, rowsAffected: 1 }) // insert priced + .mockResolvedValueOnce({ rowsAffected: 1 }); // bump updated_at + await upsertSnapshotLines(5, [ + { account_id: 1, value: 1000 }, + { + account_id: 7, + account_kind: "priced", + quantity: 10, + unit_price: 50, + value: 500, + }, + ]); + // Simple insert uses literal NULLs for qty/price + expect(mockExecute.mock.calls[1][0] as string).toMatch( + /VALUES\s*\(\s*\$1,\s*\$2,\s*NULL,\s*NULL,\s*\$3/ + ); + expect(mockExecute.mock.calls[1][1]).toEqual([5, 1, 1000]); + // Priced insert uses placeholders + expect(mockExecute.mock.calls[2][0] as string).toMatch( + /VALUES\s*\(\s*\$1,\s*\$2,\s*\$3,\s*\$4,\s*\$5/ + ); + expect(mockExecute.mock.calls[2][1]).toEqual([5, 7, 10, 50, 500]); + }); +}); diff --git a/src/services/balance.service.ts b/src/services/balance.service.ts index 7bc39bb..a3c0b63 100644 --- a/src/services/balance.service.ts +++ b/src/services/balance.service.ts @@ -36,7 +36,11 @@ export type BalanceErrorCode = | "snapshot_date_taken" | "snapshot_not_found" | "snapshot_value_invalid" - | "snapshot_priced_unsupported"; + | "snapshot_priced_unsupported" + | "snapshot_priced_quantity_required" + | "snapshot_priced_unit_price_required" + | "snapshot_priced_value_mismatch" + | "snapshot_simple_must_be_scalar"; export class BalanceServiceError extends Error { readonly code: BalanceErrorCode; @@ -528,26 +532,127 @@ export async function listLinesBySnapshot( ); } +/** + * Tolerance ε used by the priced-kind invariant `value === quantity * unit_price`. + * + * Floating-point multiplication of decimal user input is lossy + * (`12.34 * 1.07 === 13.2038000000000002`), and the UI displays `value` + * rounded to 2 decimals while keeping quantity / unit_price at full + * precision. ε = 0.01 (one cent on the dollar) is generous enough to + * absorb that drift but tight enough to catch obvious mistakes (off by + * 10×). See decisions-log.md / Issue #140. + */ +export const PRICED_VALUE_TOLERANCE = 0.01; + export interface SnapshotLineInput { account_id: number; /** - * Simple-kind value. Must be a finite number (>= 0 in practice but the - * service accepts any finite — negative values support shorts/loans). + * Snapshot value at this date. For priced lines this should match + * `quantity * unit_price` within `PRICED_VALUE_TOLERANCE`; the service + * validates the relation ahead of the SQL CHECK and surfaces a typed + * `snapshot_priced_value_mismatch` error otherwise. */ value: number; + /** + * Category kind of the underlying account. Defaults to 'simple' to + * preserve the #146 callers that don't pass it. Priced lines must + * provide both `quantity` and `unit_price`. + */ + account_kind?: BalanceCategoryKind; + /** Required for priced lines, must be NULL for simple. */ + quantity?: number | null; + /** Required for priced lines, must be NULL for simple. */ + unit_price?: number | null; } /** - * Upsert a batch of snapshot lines (simple kind only). Each input row is - * inserted or replaced atomically per account; lines for accounts not - * present in `lines` are removed from the snapshot. This makes the editor - * strictly state-driven — what the user sees is exactly what gets saved. + * Pure helper that validates a snapshot line against its account's + * category kind. Exposed for unit tests and used by `upsertSnapshotLines` + * before any DB mutation happens. * - * Validation enforced ahead of time so the SQL CHECK never fires: - * - finite numeric value (NaN / +-Infinity rejected with `snapshot_value_invalid`); - * - quantity / unit_price always stored as NULL (simple-kind invariant). + * Rules: + * - simple kind → quantity AND unit_price must be NULL/undefined; value + * must be a finite number. + * - priced kind → quantity AND unit_price must be finite numbers; value + * must equal quantity × unit_price within + * `PRICED_VALUE_TOLERANCE`. * - * Priced-kind upsert lands in Issue #140 (Bilan #2). + * @throws `BalanceServiceError` with a typed code on the first failure. + */ +export function validateLineKindInvariants( + line: SnapshotLineInput, + accountKind: BalanceCategoryKind = line.account_kind ?? "simple" +): void { + if (typeof line.value !== "number" || !Number.isFinite(line.value)) { + throw new BalanceServiceError( + "snapshot_value_invalid", + `Line for account ${line.account_id}: value must be a finite number` + ); + } + if (accountKind === "simple") { + // Simple-kind: quantity / unit_price must be absent (NULL or undefined). + if (line.quantity !== undefined && line.quantity !== null) { + throw new BalanceServiceError( + "snapshot_simple_must_be_scalar", + `Line for account ${line.account_id}: simple-kind line must not carry quantity` + ); + } + if (line.unit_price !== undefined && line.unit_price !== null) { + throw new BalanceServiceError( + "snapshot_simple_must_be_scalar", + `Line for account ${line.account_id}: simple-kind line must not carry unit_price` + ); + } + return; + } + // Priced-kind: both fields required and finite. + if ( + line.quantity === undefined || + line.quantity === null || + typeof line.quantity !== "number" || + !Number.isFinite(line.quantity) + ) { + throw new BalanceServiceError( + "snapshot_priced_quantity_required", + `Line for account ${line.account_id}: quantity is required for priced accounts` + ); + } + if ( + line.unit_price === undefined || + line.unit_price === null || + typeof line.unit_price !== "number" || + !Number.isFinite(line.unit_price) + ) { + throw new BalanceServiceError( + "snapshot_priced_unit_price_required", + `Line for account ${line.account_id}: unit_price is required for priced accounts` + ); + } + const expected = line.quantity * line.unit_price; + if (Math.abs(expected - line.value) > PRICED_VALUE_TOLERANCE) { + throw new BalanceServiceError( + "snapshot_priced_value_mismatch", + `Line for account ${line.account_id}: value ${line.value} does not match quantity × unit_price (${expected})` + ); + } +} + +/** + * Upsert a batch of snapshot lines. Each input row is inserted or + * replaced atomically per account; lines for accounts not present in + * `lines` are removed from the snapshot. This makes the editor strictly + * state-driven — what the user sees is exactly what gets saved. + * + * Validation enforced ahead of time so the SQL CHECK never fires + * (`validateLineKindInvariants`): + * - simple kind → quantity / unit_price must be NULL; value must be finite. + * - priced kind → quantity / unit_price must be finite, and + * `value === quantity * unit_price` within + * `PRICED_VALUE_TOLERANCE`. + * + * The default `account_kind = 'simple'` preserves the #146 calling + * convention — callers that pre-classify their lines (which the priced + * editor in #140 must do) pass `account_kind: 'priced'` explicitly. */ export async function upsertSnapshotLines( snapshotId: number, @@ -562,15 +667,7 @@ export async function upsertSnapshotLines( } // Validate every input up-front before mutating anything. for (const line of lines) { - if ( - typeof line.value !== "number" || - !Number.isFinite(line.value) - ) { - throw new BalanceServiceError( - "snapshot_value_invalid", - `Line for account ${line.account_id}: value must be a finite number` - ); - } + validateLineKindInvariants(line); } const db = await getDb(); @@ -582,12 +679,28 @@ export async function upsertSnapshotLines( [snapshotId] ); for (const line of lines) { - await db.execute( - `INSERT INTO balance_snapshot_lines - (snapshot_id, account_id, quantity, unit_price, value, price_source) - VALUES ($1, $2, NULL, NULL, $3, 'manual')`, - [snapshotId, line.account_id, line.value] - ); + const kind = line.account_kind ?? "simple"; + if (kind === "simple") { + await db.execute( + `INSERT INTO balance_snapshot_lines + (snapshot_id, account_id, quantity, unit_price, value, price_source) + VALUES ($1, $2, NULL, NULL, $3, 'manual')`, + [snapshotId, line.account_id, line.value] + ); + } else { + await db.execute( + `INSERT INTO balance_snapshot_lines + (snapshot_id, account_id, quantity, unit_price, value, price_source) + VALUES ($1, $2, $3, $4, $5, 'manual')`, + [ + snapshotId, + line.account_id, + line.quantity, + line.unit_price, + line.value, + ] + ); + } } // Bump the parent snapshot's updated_at so list views can sort by recency. await db.execute(