fix(billing): stop double-counting history a BALANCE FORWARD already carries
Build and Push Images / Build jorgecuadros-web (push) Successful in 1m52s
Build and Push Images / Build jorgecuadros-api (push) Successful in 2m4s

BALANCE FORWARD rows are not movements. Access materialized one per
customer per year, dated Jan 1, holding the closing balance of everything
before it — that is what let the portal keep each year in its own table and
still show a correct running balance from one year's rows.

The platform imported those rows AND the real pre-cutover history they
summarize, and every balance aggregate summed the lot. NUMid 501 read
-10,469.29 on the receivables worklist against -14,065.29 on the customer's
own statement and on the legacy portal; the gap was two cash receipts from
2009 and 2012 that the 2026 opening balance had already absorbed.

The scale settles what it is: summed the old way the whole book came to
+20,605,447.86 MXN — the office owing its customers 20.6 million pesos.
Floored, it is -56,855.90. A receivables ledger cannot be 20M in credit.

Adds BALANCE_FLOOR_JOIN + NOT_SUPERSEDED and applies them to balances()
(page and count queries, which must agree), to stats()'s per-currency and
per-domain figures, and to the owing/in-credit split. The four stats()
aggregates moved from Prisma groupBy to raw SQL because groupBy cannot
express a per-customer floor.

statement() takes the same floor as a scalar, which is also what stops
FEE ANUAL and fee15 leaking in. Those are not in
STATEMENT_EXCLUDED_SOURCE_TABLES — that list reproduces legacy's
DATOS2-only `datosfreak` — and they were putting 2,092 pre-cutover fee rows
across 1,062 customers into the statement, skewing it by -5,129,764 against
the number those customers have been quoted for years. Dating rather than
source is the right test: a fee row *after* the opening balance is a real
charge and still counts.

movements() is deliberately left alone. It is a browser over captured rows
— "how much water did we capture in April" — and staff need the historical
rows visible, so it keeps totalling everything, the same asymmetry
NOT_OUTSTANDING already has.

stats() now separates the two questions it was mixing: movements,
ledgerCustomers, crossLineCustomers and the date range stay unfloored
inventory; everything under byCurrency/byDomain is a balance and is floored.

BillingService had no tests. Adds 13 covering the floor's failure modes —
it fails silently, so MIN-vs-MAX, `>` vs `>=`, the NULL branch for customers
with no opening balance, and the join/predicate alias pairing are each
pinned, plus the 501 arithmetic as a regression.

Verified through the real service against the live ledger: balances() and
statement() both return -14,065.29 for 501, matching the portal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-05 00:34:53 -07:00
co-authored by Claude Opus 5
parent e9a5ee9e90
commit d173c9e9a0
2 changed files with 339 additions and 52 deletions
+180
View File
@@ -0,0 +1,180 @@
import { Prisma } from "@jorgecuadros/database";
import {
BALANCE_FLOOR_JOIN,
BALANCE_FORWARD_TYPE,
BillingService,
NOT_SUPERSEDED,
} from "./billing.service";
/**
* The balance floor drops rows a later BALANCE FORWARD already accounts for.
*
* It is worth testing because it fails silently: nothing throws, the numbers are
* just wrong, and they were wrong for years — the whole book read +20.6M MXN in
* credit because every customer's pre-cutover history was counted twice, once
* inside their opening balance and once as itself.
*/
describe("balance floor", () => {
describe("SQL fragments", () => {
it("binds the type name rather than interpolating it", () => {
// A literal would be a second place to edit if the label ever changes,
// and this string reaches SQL from a module constant.
expect(BALANCE_FLOOR_JOIN.values).toEqual([BALANCE_FORWARD_TYPE]);
});
it("keys the floor to the row's own customer", () => {
// Without this the derived table cross-joins and every customer inherits
// the earliest BALANCE FORWARD in the book.
expect(BALANCE_FLOOR_JOIN.sql).toContain(
"bfloor ON bfloor.customerId = t.customerId",
);
});
it("takes the most recent opening balance, not the first", () => {
// A customer accumulates one BALANCE FORWARD per year. MIN would floor at
// the oldest and leave every intervening year double-counted.
expect(BALANCE_FLOOR_JOIN.sql).toContain("MAX(bf.transactionDate)");
expect(BALANCE_FLOOR_JOIN.sql).not.toContain("MIN(bf.transactionDate)");
});
it("ignores voided opening balances when locating the floor", () => {
expect(BALANCE_FLOOR_JOIN.sql).toContain("bf.voidedAt IS NULL");
});
it("is inclusive of the opening balance row itself", () => {
// `>` instead of `>=` would drop the carried balance and understate every
// customer by exactly that amount.
expect(NOT_SUPERSEDED.sql).toContain("t.transactionDate >= bfloor.floorDate");
expect(NOT_SUPERSEDED.sql).not.toMatch(/transactionDate\s*>\s*bfloor/);
});
it("leaves customers with no opening balance untouched", () => {
// NULL comparisons are never true, so without the explicit IS NULL branch
// a customer who has no BALANCE FORWARD row loses their entire ledger.
expect(NOT_SUPERSEDED.sql).toContain("bfloor.floorDate IS NULL");
});
it("only ever references the alias the join defines", () => {
// The predicate is useless without the join; pairing them wrongly is a
// runtime "unknown column", so keep the alias identical in both.
const aliases = NOT_SUPERSEDED.sql.match(/bfloor\.\w+/g) ?? [];
expect(aliases.length).toBeGreaterThan(0);
for (const ref of aliases) {
expect(BALANCE_FLOOR_JOIN.sql).toContain(ref.split(".")[1]);
}
});
});
describe("statement()", () => {
/**
* One customer means one floor date, so the statement uses a scalar lookup
* instead of the join. Asserting on the `where` Prisma is handed is the only
* way to see it without a database.
*/
function serviceWith(floor: Date | null) {
const findMany = jest.fn().mockResolvedValue([]);
const prisma = {
customer: {
findUnique: jest.fn().mockResolvedValue({
id: "c1",
name: "CUADROS, JORGE H.",
preferredCurrency: "USD",
_count: { properties: 0, policies: 0 },
}),
},
transaction: {
findFirst: jest
.fn()
.mockResolvedValue(floor ? { transactionDate: floor } : null),
findMany,
},
};
return {
service: new BillingService(prisma as never),
prisma,
findMany,
};
}
it("looks the floor up from the customer's newest opening balance", async () => {
const { service, prisma } = serviceWith(new Date("2026-01-01T00:00:00Z"));
await service.statement("c1");
expect(prisma.transaction.findFirst).toHaveBeenCalledWith(
expect.objectContaining({
where: {
customerId: "c1",
voidedAt: null,
type: { nameEn: BALANCE_FORWARD_TYPE },
},
orderBy: { transactionDate: "desc" },
select: { transactionDate: true },
}),
);
});
it("bounds the statement at the floor, inclusive", async () => {
const floor = new Date("2026-01-01T00:00:00Z");
const { service, findMany } = serviceWith(floor);
await service.statement("c1");
expect(findMany.mock.calls[0][0].where).toMatchObject({
customerId: "c1",
transactionDate: { gte: floor },
});
});
it("applies no date bound when the customer has no opening balance", async () => {
const { service, findMany } = serviceWith(null);
await service.statement("c1");
expect(findMany.mock.calls[0][0].where).not.toHaveProperty(
"transactionDate",
);
});
it("keeps the source-table exclusion alongside the floor", async () => {
// The two guards answer different questions — one reproduces legacy's
// DATOS2-only materialization, the other drops superseded history — and
// dropping either one changes the customer's balance.
const { service, findMany } = serviceWith(new Date("2026-01-01T00:00:00Z"));
await service.statement("c1");
const where = findMany.mock.calls[0][0].where;
expect(where.OR).toEqual([
{ legacySourceTable: null },
{ legacySourceTable: { notIn: expect.arrayContaining(["EFECTIVO"]) } },
]);
});
});
describe("regression: NUMid 501", () => {
/**
* The arithmetic that exposed the bug, pinned so it cannot silently return.
* Figures measured against the live ledger on 2026-08-05.
*/
const openingBalance = new Prisma.Decimal("-6732.29");
const activitySinceOpening = new Prisma.Decimal("-7333.00");
const preCutoverCashAlreadyInOpening = new Prisma.Decimal("3596.00");
it("matches the legacy portal once superseded rows are dropped", () => {
expect(openingBalance.plus(activitySinceOpening).toFixed(2)).toBe(
"-14065.29",
);
});
it("reproduces the wrong figure when they are not", () => {
expect(
openingBalance
.plus(activitySinceOpening)
.plus(preCutoverCashAlreadyInOpening)
.toFixed(2),
).toBe("-10469.29");
});
});
});
+159 -52
View File
@@ -104,24 +104,31 @@ interface BalanceRow {
nameMissing: number; nameMissing: number;
city: string | null; city: string | null;
state: string | null; state: string | null;
movements: bigint | number | string; movements: RawCount;
balanceMxn: Prisma.Decimal | null; balanceMxn: Prisma.Decimal | null;
balanceUsd: Prisma.Decimal | null; balanceUsd: Prisma.Decimal | null;
chargesMxn: Prisma.Decimal | null; chargesMxn: Prisma.Decimal | null;
creditsMxn: Prisma.Decimal | null; creditsMxn: Prisma.Decimal | null;
chargesUsd: Prisma.Decimal | null; chargesUsd: Prisma.Decimal | null;
creditsUsd: Prisma.Decimal | null; creditsUsd: Prisma.Decimal | null;
utilityMovements: bigint | number | string; utilityMovements: RawCount;
insuranceMovements: bigint | number | string; insuranceMovements: RawCount;
lastMovement: Date | null; lastMovement: Date | null;
} }
/** /**
* Raw-query counts come back in three shapes depending on the aggregate: * Every shape a raw-query count can arrive in. `COUNT(*)` is a bigint,
* `COUNT(*)` as bigint, `SUM(bool)` as a decimal *string*, and plain numbers. * `SUM(bool)` is a Prisma.Decimal, and plain numbers occur too — none of which
* Normalize all of them before they reach the client as JSON. * survive JSON serialization the way the client expects.
*/ */
function num(v: bigint | number | string | null | undefined): number { type RawCount = bigint | number | string | Prisma.Decimal;
/**
* Normalizes a raw-query count before it reaches the client as JSON. A bigint
* throws on JSON.stringify and a Decimal serializes to a *string*, so counts
* must not be passed through untouched.
*/
function num(v: RawCount | null | undefined): number {
if (v === null || v === undefined) return 0; if (v === null || v === undefined) return 0;
return typeof v === "number" ? v : Number(v); return typeof v === "number" ? v : Number(v);
} }
@@ -152,6 +159,56 @@ const NOT_VOIDED: Prisma.TransactionWhereInput = { voidedAt: null };
*/ */
const NOT_OUTSTANDING: Prisma.TransactionWhereInput = { outstanding: false }; const NOT_OUTSTANDING: Prisma.TransactionWhereInput = { outstanding: false };
/**
* The legacy type name for a carried-forward opening balance.
*
* These rows are not movements. Access materialized one per customer per year,
* dated Jan 1, holding the closing balance of everything before it — that is
* what let the portal keep each year in its own table (`datosfreak` = current,
* `2025`, `2024`, ...) and still show a correct running balance from a single
* year's rows.
*/
export const BALANCE_FORWARD_TYPE = "BALANCE FORWARD";
/**
* Per-customer date of the most recent BALANCE FORWARD row.
*
* Joined rather than correlated: one small derived table (1,170 rows) beats a
* subquery evaluated per ledger row.
*/
export const BALANCE_FLOOR_JOIN = Prisma.sql`
LEFT JOIN (
SELECT bf.customerId, MAX(bf.transactionDate) AS floorDate
FROM transactions bf
JOIN type_transactions bft ON bft.id = bf.typeId
WHERE bft.nameEn = ${BALANCE_FORWARD_TYPE} AND bf.voidedAt IS NULL
GROUP BY bf.customerId
) bfloor ON bfloor.customerId = t.customerId`;
/**
* Excludes rows a later BALANCE FORWARD already accounts for.
*
* WHY THIS EXISTS. The platform holds both the synthetic BALANCE FORWARD rows
* and the real pre-cutover history they summarize, so summing a customer's
* whole ledger counts that history twice — once inside the opening balance,
* once as itself. NUMid 501 read -10,469.29 on the worklist against -14,065.29
* on the customer's own statement and on the legacy portal, the gap being two
* cash receipts from 2009 and 2012 that the 2026 opening balance had already
* absorbed.
*
* The scale is what settles it: summed the old way the entire book came to
* +20,605,447.86 MXN — the office owing its customers 20.6 million pesos.
* Floored, it is -56,855.90, a modest net receivable. A receivables ledger
* cannot be 20M in credit.
*
* Applies to BALANCES ONLY, in the same spirit as NOT_OUTSTANDING: the movement
* browser still totals every captured row, because "how much water did we
* capture in April" is a question about what was recorded, not about what is
* owed. Customers with no BALANCE FORWARD row (the floor is NULL) are
* unaffected.
*/
export const NOT_SUPERSEDED = Prisma.sql`(bfloor.floorDate IS NULL OR t.transactionDate >= bfloor.floorDate)`;
/** /**
* Source tables excluded from the customer-facing statement. * Source tables excluded from the customer-facing statement.
* *
@@ -402,22 +459,24 @@ export class BillingService {
MAX(t.transactionDate) AS lastMovement MAX(t.transactionDate) AS lastMovement
FROM customers c FROM customers c
JOIN transactions t ON t.customerId = c.id JOIN transactions t ON t.customerId = c.id
WHERE t.voidedAt IS NULL AND t.outstanding = 0 ${nameFilter} ${txFilter} ${BALANCE_FLOOR_JOIN}
WHERE t.voidedAt IS NULL AND t.outstanding = 0 AND ${NOT_SUPERSEDED} ${nameFilter} ${txFilter}
GROUP BY c.id, c.name, c.nameSource, c.nameMissing, c.city, c.state GROUP BY c.id, c.name, c.nameSource, c.nameMissing, c.city, c.state
${having} ${having}
${orderBy} ${orderBy}
LIMIT ${pageSize} OFFSET ${(page - 1) * pageSize} LIMIT ${pageSize} OFFSET ${(page - 1) * pageSize}
`; `;
const counted = await this.prisma.$queryRaw<{ total: bigint | number | string }[]>` const counted = await this.prisma.$queryRaw<{ total: RawCount }[]>`
SELECT COUNT(*) AS total FROM ( SELECT COUNT(*) AS total FROM (
SELECT c.id SELECT c.id
FROM customers c FROM customers c
JOIN transactions t ON t.customerId = c.id JOIN transactions t ON t.customerId = c.id
${BALANCE_FLOOR_JOIN}
-- Must match the page query's filters exactly, or the total disagrees -- Must match the page query's filters exactly, or the total disagrees
-- with the rows. (The void exclusion was missing here before the -- with the rows. (The void exclusion was missing here before the
-- outstanding work; a voided-only customer inflated the count.) -- outstanding work; a voided-only customer inflated the count.)
WHERE t.voidedAt IS NULL AND t.outstanding = 0 ${nameFilter} ${txFilter} WHERE t.voidedAt IS NULL AND t.outstanding = 0 AND ${NOT_SUPERSEDED} ${nameFilter} ${txFilter}
GROUP BY c.id GROUP BY c.id
${having} ${having}
) x ) x
@@ -458,9 +517,18 @@ export class BillingService {
}; };
} }
/** Top-line figures for the billing page header. */ /**
* Top-line figures for the billing page header.
*
* Two different questions live here and they use different row sets.
* `movements`, `ledgerCustomers`, `crossLineCustomers` and the date range are
* INVENTORY — what is stored — and count everything not voided. Everything
* under `byCurrency` / `byDomain` is a BALANCE, so it applies NOT_SUPERSEDED
* and drops rows an opening balance already accounts for. The four aggregates
* moved from Prisma groupBy to raw SQL to express that join; groupBy cannot.
*/
async stats() { async stats() {
const [movements, ledgerCustomers, byCurrency, byDomain] = await Promise.all([ const [movements, ledgerCustomers] = await Promise.all([
this.prisma.transaction.count({ where: NOT_VOIDED }), this.prisma.transaction.count({ where: NOT_VOIDED }),
this.prisma.transaction this.prisma.transaction
.findMany({ .findMany({
@@ -469,34 +537,47 @@ export class BillingService {
select: { customerId: true }, select: { customerId: true },
}) })
.then((r) => r.length), .then((r) => r.length),
this.prisma.transaction.groupBy({
by: ["currency"],
where: NOT_VOIDED,
_sum: { amount: true },
_count: { _all: true },
}),
this.prisma.transaction.groupBy({
by: ["domain", "currency"],
where: NOT_VOIDED,
_sum: { amount: true },
_count: { _all: true },
}),
]); ]);
const charges = await this.prisma.transaction.groupBy({ const byCurrency = await this.prisma.$queryRaw<
by: ["currency"], {
where: { AND: [{ amount: { lt: 0 } }, NOT_VOIDED] }, currency: string;
_sum: { amount: true }, net: Prisma.Decimal | null;
_count: { _all: true }, count: RawCount;
}); charges: Prisma.Decimal | null;
const credits = await this.prisma.transaction.groupBy({ chargeCount: RawCount;
by: ["currency"], credits: Prisma.Decimal | null;
where: { AND: [{ amount: { gt: 0 } }, NOT_VOIDED] }, creditCount: RawCount;
_sum: { amount: true }, }[]
_count: { _all: true }, >`
}); SELECT t.currency AS currency,
const chargeMap = new Map(charges.map((c) => [c.currency, c])); SUM(t.amount) AS net,
const creditMap = new Map(credits.map((c) => [c.currency, c])); COUNT(*) AS count,
SUM(CASE WHEN t.amount < 0 THEN t.amount ELSE 0 END) AS charges,
SUM(t.amount < 0) AS chargeCount,
SUM(CASE WHEN t.amount > 0 THEN t.amount ELSE 0 END) AS credits,
SUM(t.amount > 0) AS creditCount
FROM transactions t
${BALANCE_FLOOR_JOIN}
WHERE t.voidedAt IS NULL AND ${NOT_SUPERSEDED}
GROUP BY t.currency
`;
const byDomain = await this.prisma.$queryRaw<
{
domain: string;
currency: string;
net: Prisma.Decimal | null;
count: RawCount;
}[]
>`
SELECT t.domain AS domain, t.currency AS currency,
SUM(t.amount) AS net, COUNT(*) AS count
FROM transactions t
${BALANCE_FLOOR_JOIN}
WHERE t.voidedAt IS NULL AND ${NOT_SUPERSEDED}
GROUP BY t.domain, t.currency
`;
// How many customers sit on each side of the line, per currency — the // How many customers sit on each side of the line, per currency — the
// headline for a receivables view. Counted in SQL; a customer can be // headline for a receivables view. Counted in SQL; a customer can be
@@ -504,16 +585,19 @@ export class BillingService {
const sides = await this.prisma.$queryRaw< const sides = await this.prisma.$queryRaw<
{ {
currency: string; currency: string;
owing: bigint | number | string; owing: RawCount;
inCredit: bigint | number | string; inCredit: RawCount;
}[] }[]
>` >`
SELECT currency, SELECT currency,
SUM(bal < -0.005) AS owing, SUM(bal < -0.005) AS owing,
SUM(bal > 0.005) AS inCredit SUM(bal > 0.005) AS inCredit
FROM ( FROM (
SELECT customerId, currency, SUM(amount) AS bal SELECT t.customerId, t.currency, SUM(t.amount) AS bal
FROM transactions WHERE voidedAt IS NULL GROUP BY customerId, currency FROM transactions t
${BALANCE_FLOOR_JOIN}
WHERE t.voidedAt IS NULL AND ${NOT_SUPERSEDED}
GROUP BY t.customerId, t.currency
) x ) x
GROUP BY currency GROUP BY currency
`; `;
@@ -534,7 +618,7 @@ export class BillingService {
// Customers whose ledger spans both business lines — the whole reason this // Customers whose ledger spans both business lines — the whole reason this
// module is one view instead of two. // module is one view instead of two.
const crossLine = await this.prisma.$queryRaw<{ n: bigint | number | string }[]>` const crossLine = await this.prisma.$queryRaw<{ n: RawCount }[]>`
SELECT COUNT(*) AS n FROM ( SELECT COUNT(*) AS n FROM (
SELECT customerId FROM transactions WHERE voidedAt IS NULL SELECT customerId FROM transactions WHERE voidedAt IS NULL
GROUP BY customerId HAVING COUNT(DISTINCT domain) > 1 GROUP BY customerId HAVING COUNT(DISTINCT domain) > 1
@@ -549,20 +633,20 @@ export class BillingService {
lastMovement: lastRow?.transactionDate ?? null, lastMovement: lastRow?.transactionDate ?? null,
byCurrency: byCurrency.map((c) => ({ byCurrency: byCurrency.map((c) => ({
currency: c.currency, currency: c.currency,
net: c._sum.amount, net: c.net,
count: c._count._all, count: num(c.count),
charges: chargeMap.get(c.currency)?._sum.amount ?? null, charges: c.charges,
chargeCount: chargeMap.get(c.currency)?._count._all ?? 0, chargeCount: num(c.chargeCount),
credits: creditMap.get(c.currency)?._sum.amount ?? null, credits: c.credits,
creditCount: creditMap.get(c.currency)?._count._all ?? 0, creditCount: num(c.creditCount),
owing: num(sideMap.get(c.currency)?.owing), owing: num(sideMap.get(c.currency)?.owing),
inCredit: num(sideMap.get(c.currency)?.inCredit), inCredit: num(sideMap.get(c.currency)?.inCredit),
})), })),
byDomain: byDomain.map((d) => ({ byDomain: byDomain.map((d) => ({
domain: d.domain, domain: d.domain,
currency: d.currency, currency: d.currency,
net: d._sum.amount, net: d.net,
count: d._count._all, count: num(d.count),
})), })),
}; };
} }
@@ -589,7 +673,7 @@ export class BillingService {
}); });
const years = await this.prisma.$queryRaw< const years = await this.prisma.$queryRaw<
{ year: number; count: bigint | number | string }[] { year: number; count: RawCount }[]
>` >`
SELECT YEAR(transactionDate) AS year, COUNT(*) AS count SELECT YEAR(transactionDate) AS year, COUNT(*) AS count
FROM transactions WHERE voidedAt IS NULL GROUP BY year ORDER BY year DESC FROM transactions WHERE voidedAt IS NULL GROUP BY year ORDER BY year DESC
@@ -647,9 +731,32 @@ export class BillingService {
throw new NotFoundException(`Customer ${customerId} not found`); throw new NotFoundException(`Customer ${customerId} not found`);
} }
// One customer, so the balance floor is a single date rather than the
// derived table the aggregate queries join. See NOT_SUPERSEDED: rows before
// the opening balance are already inside it, and showing them would both
// double the total and make every balanceAfter below wrong.
//
// This is also what stops FEE ANUAL and fee15 leaking in. They are not in
// STATEMENT_EXCLUDED_SOURCE_TABLES — that list exists to reproduce legacy's
// DATOS2-only `datosfreak`, and it was letting 2,092 pre-cutover fee rows
// across 1,062 customers through, skewing the statement by -5,129,764
// against the number those customers have been quoted for years. Dating
// rather than source is the right test: a FEE ANUAL row *after* the opening
// balance is a real charge and still counts.
const floor = await this.prisma.transaction.findFirst({
where: {
customerId,
voidedAt: null,
type: { nameEn: BALANCE_FORWARD_TYPE },
},
orderBy: { transactionDate: "desc" },
select: { transactionDate: true },
});
const rows = await this.prisma.transaction.findMany({ const rows = await this.prisma.transaction.findMany({
where: { where: {
customerId, customerId,
...(floor ? { transactionDate: { gte: floor.transactionDate } } : {}),
// NULL-safe exclusion. `notIn` alone compiles to SQL `NOT IN`, and // NULL-safe exclusion. `notIn` alone compiles to SQL `NOT IN`, and
// `NULL NOT IN (...)` is NULL, not true — so every app-captured row // `NULL NOT IN (...)` is NULL, not true — so every app-captured row
// (which has no legacySourceTable) silently vanished from the // (which has no legacySourceTable) silently vanished from the