fix(qa): cent-exact toCents rounding + money.js test coverage

- utils/money: toCents rounds off the shortest decimal string instead of
  Math.round(n*100), so 1.005 -> 101 (not 100). Output is identical for all
  integer / <=2-decimal / "$1,234.56" inputs, so no downstream change (QA-B7-01)
- add tests/money.test.js (9 tests; the money core previously had none)
- docs: archive QA-B7-01 to HISTORY v0.41.0; QA cycle 1 now 0 open findings

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
null 2026-07-02 21:11:12 -05:00
parent 98c8fab176
commit 72d06aa2d8
4 changed files with 97 additions and 25 deletions

View File

@ -7,6 +7,7 @@
- **[Bills] `expected_amount` was unvalidated** — POST/PUT `/api/bills` accepted negative amounts, non-numeric input (silently coerced to $0), and absurd values (`1e15` → cents past `Number.MAX_SAFE_INTEGER`). `validateBillData` (`services/billsService.js`) now rejects non-numeric / negative / out-of-range with a structured `VALIDATION_ERROR`, accepting `0`…`$100,000,000`. Regression assertions in `e2e/api.probe.spec.js`. (was QA-B13-01)
- **[UI] Negative amounts rendered as "$-50.00"** — client `fmt()` (`client/lib/utils.js`) and server `formatUSD()` (`utils/money.js`) placed the minus sign after the currency symbol; now render the conventional "-$50.00". Test added in `client/lib/utils.test.js`. (was QA-B6-01)
- **[a11y] Icon-only controls and chart SVGs had no accessible name** — Radix Select filter/sort triggers (Tracker, Bills) and the Spending month-nav buttons rendered with no discernible text (screen readers announced a bare "button"); Analytics chart `<svg role="img">`s had no name; a Snowball drag-handle `<div>` used a prohibited `aria-label`. Added `aria-label`s to the triggers/buttons and a `label` prop to the Analytics `SvgFrame`, and switched the drag-handle to `title`. Clears axe **critical `button-name`** and **serious `svg-img-alt` / `aria-prohibited-attr`** across those pages; guarded by `e2e/a11y.authed.spec.js`. (was QA-B14-01, part of QA-B14-02)
- **[Money] `toCents` lost a cent on 3-decimal half values** — `toCents(1.005)` returned `100` ($1.00) instead of `101`, because `Math.round(n * 100)` inherits binary-float error (`1.005 * 100 === 100.4999…`). Now rounds off the shortest decimal string that round-trips the value (half away from zero); output is **identical** for every integer / ≤2-decimal / `"$1,234.56"` input, so no downstream behavior changes. Added `tests/money.test.js` (9 tests). (was QA-B7-01)
- **[a11y] Nested interactive controls in Categories & Snowball rows** — Categories rows were `role="button"` (and draggable) yet wrapped their own move/edit/delete buttons, and the Snowball plan-status header wrapped its action buttons *inside* the collapsible trigger button (axe **serious `nested-interactive`**). Restructured both: Categories now use a plain container with a dedicated chevron toggle button (click-anywhere still expands via mouse; keyboard/SR use the chevron); the Snowball header splits into a name/progress toggle, sibling action buttons, and a chevron toggle. Expand/collapse verified by `e2e/categories.spec.js`; all 8 authenticated pages now pass axe. (completes QA-B14-02)
### 🧹 QA Cleanup

View File

@ -94,7 +94,7 @@ before cross-cutting; regression last). Update **Status** and **Findings** every
| B4 | Subscriptions & Categories | `/subscriptions`, catalog, `/categories`, groups, reorder | seeded | ⬜ | 0 / 0 |
| B5 | Reporting reconciliation | `/summary`, `/calendar`, `/analytics`, `/health` cross-check totals | seeded + large | ⬜ | 0 / 0 |
| B6 | Spending | `/spending` YNAB view, averages, cover-overspending, safe-to-spend | seeded + edge months | 🔄 | 0 / 1 |
| B7 | Debt planning (math) | `/snowball`, `/payoff` APR/amortization vs hand-calc | edge (APR=0, $0 debt) | 🔄 | 1 / 1 |
| B7 | Debt planning (math) | `/snowball`, `/payoff` APR/amortization vs hand-calc | edge (APR=0, $0 debt) | 🔄 | 0 / 2 |
| B8 | Banking & bank sync | `/bank-transactions`, SimpleFIN sync, matching, merchant/store, advisory filter | seeded txns | ⬜ | 0 / 0 |
| B9 | Data lifecycle | `/data` import (XLSX/CSV/SQLite), export, ICS feed, backups round-trip | empty + seeded | 🔄 | 0 / 1 |
| B10 | Notifications & workers | email + ntfy/Gotify/Discord/Telegram, reminders, cron workers | seeded | ⬜ | 0 / 0 |
@ -115,7 +115,7 @@ until you get a clean cycle.
| Cycle | Started | Build / commit | Findings logged | Fixed / archived | Result |
|-------|---------|----------------|-----------------|------------------|--------|
| 1 | 2026-07-02 | `bdbf231` (dev) | 9 (find pass ongoing) | 8 → HISTORY v0.41.0 (B9-01, B13-01, B6-01, B14-01, B14-02, B14-03, B0-01, B7-02) | 🔄 in progress — B0/B1/B3/B4/B6/B7/B8/B9/B13/B14 probed. Solid: auth-isolation, CSRF, payment/date validation, **recurrence (quarterly/annual gating, Feb-31 clamp, leap year)**, **transaction matching/dedup**, subscription+spending math, XSS. **Fixed: seed 100× cents (S2), bill-amount validation, negative-money format, all a11y (button-name/svg/aria/nested-interactive — 8/8 pages pass axe), vendor-bundle split, unused-dep + dead-code removal.** Open: 1 (B7-01 rounding S3 [float-inherent, deferred]) |
| 1 | 2026-07-02 | `bdbf231`→`98c8fab` (dev) | 9 | **9 → all fixed & archived** (B9-01, B13-01, B6-01, B7-01, B7-02, B14-01, B14-02, B14-03, B0-01) | 🔁 all findings fixed — **0 open**; re-run required for a clean pass. Probed B0/B1/B3/B4/B6/B7/B8/B9/B13/B14. Solid: auth-isolation, CSRF, payment/date validation, recurrence (quarterly/annual gating, Feb-31, leap year), transaction matching/dedup, subscription+spending math, XSS. Fixed: seed 100× cents (S2), bill-amount validation, both money-rounding/format bugs, all a11y (8/8 axe), bundle split, unused-dep + dead-code removal. |
**Result key:** 🔄 in progress · 🔁 findings fixed, re-run required · ✅ clean (zero findings — QA complete)
@ -134,7 +134,7 @@ fixing. Keep only **Open / Fixing / Fixed** rows here. Once a finding is
| ID | Sev | Area (`file:line`) | Summary | Status | Notes / repro |
|----|-----|--------------------|---------|--------|---------------|
| QA-B7-01 | S3 | `utils/money.js:29` | `toCents` mis-rounds fractional cents: `toCents(1.005)` → 100 (`$1.00`) not 101 | 🔴 Open | see write-up (deferred — float-inherent) |
| _(none — all Cycle 1 findings fixed & archived to `HISTORY.md` v0.41.0)_ | | | | | |
**Finding template** (paste a new row above; keep the full write-up here until archived):
@ -155,24 +155,7 @@ Fix: (what changed, commit) — Verified by: (repro re-run + ci)
Log console errors, failed network requests, and unhandled rejections as findings
**even if the UI looks fine**.
### Cycle 1 — logged write-ups
```
ID: QA-B7-01
Severity: S3 (minor — wrong edge behavior in money core that advertises exactness)
Environment: server-side money math
Area: utils/money.js:29 (toCents → Math.round(n * 100))
Steps to reproduce:
1. toCents(1.005) → 100 (i.e. $1.00), not 101 ($1.01).
2. Round-trip fromCents(toCents(1.005)) → 1 (a cent silently lost).
Expected: "cent-exact" per the file's own docstring — 1.005 → 101.
Actual: float multiply (1.005*100 = 100.4999…) rounds down before Math.round.
Evidence: node probe. Other 3-decimal inputs also affected (values near .xx5).
Impact: bounded to sub-cent, and only when a 3+ decimal dollar value reaches the
boundary (proration/interest), so low severity — but it contradicts the exactness
guarantee and is the plan's named "fractional cents" adversarial case.
Fix (deferred): round on a string/scaled-integer basis, or add epsilon before round.
```
_All Cycle 1 write-ups have been archived to `HISTORY.md` v0.41.0 (see §3)._
---

73
tests/money.test.js Normal file
View File

@ -0,0 +1,73 @@
'use strict';
const { test } = require('node:test');
const assert = require('node:assert');
const { toCents, fromCents, roundMoney, sumMoney, mulMoney, formatUSD, formatCentsUSD } = require('../utils/money');
test('toCents — unchanged for integer / ≤2-decimal / formatted inputs', () => {
assert.strictEqual(toCents(0), 0);
assert.strictEqual(toCents(85), 8500);
assert.strictEqual(toCents(19.99), 1999);
assert.strictEqual(toCents(100.001), 10000); // 3rd decimal <5 rounds down (10000.1)
assert.strictEqual(toCents('$1,234.56'), 123456);
assert.strictEqual(toCents('9,999,999.99'), 999999999);
assert.strictEqual(toCents(0.1), 10);
assert.strictEqual(toCents(0.1 + 0.2), 30); // 0.30000000000000004 → 30, not 30.000…
});
test('toCents — QA-B7-01: fractional half-cents round half away from zero (was buggy)', () => {
assert.strictEqual(toCents(1.005), 101); // was 100 with Math.round(n*100)
assert.strictEqual(toCents(2.675), 268); // was 267
assert.strictEqual(toCents(0.005), 1);
assert.strictEqual(toCents('1.005'), 101);
assert.strictEqual(toCents(1.004), 100);
});
test('toCents — negatives and nullish/invalid', () => {
assert.strictEqual(toCents(-50), -5000);
assert.strictEqual(toCents('-12.34'), -1234);
assert.strictEqual(toCents(null), null);
assert.strictEqual(toCents(undefined), null);
assert.strictEqual(toCents(''), null);
assert.ok(Number.isNaN(toCents('abc')));
});
test('toCents — round-trips through fromCents for money values', () => {
for (const v of [0, 85, 19.99, 1234.56, 0.01]) {
assert.strictEqual(fromCents(toCents(v)), v);
}
});
test('fromCents', () => {
assert.strictEqual(fromCents(8500), 85);
assert.strictEqual(fromCents(null), null);
assert.strictEqual(fromCents(undefined), null);
});
test('sumMoney — cent-exact, no float drift', () => {
assert.strictEqual(sumMoney([0.1, 0.2]), 0.3);
assert.strictEqual(sumMoney([{ a: 1.11 }, { a: 2.22 }], (r) => r.a), 3.33);
assert.strictEqual(sumMoney([]), 0);
});
test('mulMoney — rounds to the cent', () => {
assert.strictEqual(mulMoney(100, 0.1), 10);
assert.strictEqual(mulMoney(19.99, 2), 39.98);
assert.strictEqual(mulMoney('bad', 2), 0);
});
test('roundMoney', () => {
assert.strictEqual(roundMoney(1.005), 1.01); // benefits from the toCents fix
assert.strictEqual(roundMoney(19.994), 19.99);
assert.strictEqual(roundMoney('abc'), 0);
});
test('formatUSD / formatCentsUSD — negative sign before the symbol (QA-B6-01)', () => {
assert.strictEqual(formatUSD(50), '$50.00');
assert.strictEqual(formatUSD(-50), '-$50.00');
assert.strictEqual(formatUSD(-1234.5), '-$1,234.50');
assert.strictEqual(formatUSD(0), '$0.00');
assert.strictEqual(formatUSD(null), '$0.00');
assert.strictEqual(formatCentsUSD(123456), '$1,234.56');
assert.strictEqual(formatCentsUSD(-5000), '-$50.00');
});

View File

@ -19,14 +19,29 @@
/**
* Dollars (number or string like "$1,234.56") integer cents.
* null/undefined/'' null. Unparseable input NaN (caller validates).
*
* Rounds off the decimal string (via the shortest round-trip representation for
* numbers) rather than `Math.round(n * 100)`, whose binary-float error rounds
* e.g. 1.005 down to 100 instead of 101 (QA-B7-01). Output is identical to the
* old helper for all integer / 2-decimal / "$1,234.56" inputs; only 3+-decimal
* half-cent values change, and they now round half away from zero.
*/
function toCents(dollars) {
if (dollars === null || dollars === undefined || dollars === '') return null;
const n = typeof dollars === 'string'
? Number(dollars.replace(/[$,\s]/g, ''))
: Number(dollars);
const cleaned = typeof dollars === 'string' ? dollars.replace(/[$,\s]/g, '') : dollars;
const n = Number(cleaned);
if (!Number.isFinite(n)) return NaN;
return Math.round(n * 100);
// The shortest decimal string that round-trips to this float (e.g. 1.005 for
// the number 1.005, not 1.00499999…). Scientific notation → float fallback.
const decimal = typeof cleaned === 'string' ? cleaned : n.toString();
if (/[eE]/.test(decimal)) return Math.round(n * 100);
const negative = decimal.trim().startsWith('-');
const [intPart, fracPart = ''] = decimal.replace('-', '').split('.');
const frac3 = (fracPart + '000').slice(0, 3);
const cents = Number(intPart || '0') * 100 + Number(frac3.slice(0, 2)) + (Number(frac3[2]) >= 5 ? 1 : 0);
return negative ? -cents : cents;
}
/** Integer cents → dollar number (for API payloads). null/undefined → null. */