From 72d06aa2d8312f9a50507d01fb8412e2d440c2fa Mon Sep 17 00:00:00 2001 From: null Date: Thu, 2 Jul 2026 21:11:12 -0500 Subject: [PATCH] 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 --- HISTORY.md | 1 + docs/QA_PLAN.md | 25 +++------------- tests/money.test.js | 73 +++++++++++++++++++++++++++++++++++++++++++++ utils/money.js | 23 +++++++++++--- 4 files changed, 97 insertions(+), 25 deletions(-) create mode 100644 tests/money.test.js diff --git a/HISTORY.md b/HISTORY.md index 5b2aedc..c79320f 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -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 ``s had no name; a Snowball drag-handle `
` 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 diff --git a/docs/QA_PLAN.md b/docs/QA_PLAN.md index a5a1560..0dd7e44 100644 --- a/docs/QA_PLAN.md +++ b/docs/QA_PLAN.md @@ -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)._ --- diff --git a/tests/money.test.js b/tests/money.test.js new file mode 100644 index 0000000..acb3857 --- /dev/null +++ b/tests/money.test.js @@ -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'); +}); diff --git a/utils/money.js b/utils/money.js index fe72708..a57dd4c 100644 --- a/utils/money.js +++ b/utils/money.js @@ -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. */