[IAmJSD]

Money was printed three ways (with the cost period set to yearly, Opportunities showed "$123456" with no separators), absent values used --, — and - depending on the screen (an empty Home mixed a green -- with a bold —), and every money column was left-aligned so digits didn't line up. This PR routes the Opportunities period figures through formatCurrency, uses one — placeholder for absent values, and right-aligns count and money columns with tabular digits on Opportunities, Purchase History and the approval queue.

Refs #739
Partial: modules/savings-history.ts (its private compact formatter and its --) is left to #749, which owns that code, and right-alignment is not yet applied to the Inventory, Plans and RI Exchange tables.

What changed

• frontend/src/utils.ts: new exported NO_VALUE (—); formatCurrency returns it for null/NaN/Infinity.
• frontend/src/recommendations.ts: a small formatScaled helper sends formatCostForPeriod and formatScaledRange through formatCurrency with the existing per-period precision (yearly 0, daily/monthly 2 and 0, hourly 4), so every period groups thousands. Opportunities headers and cells for COLUMN_DEFS kind numeric (count, savings, upfront, monthly, on-demand, effective %) get class num.
• dashboard.ts, history.ts, plans.ts, recommendations.ts, approval-details.ts: every -- and - placeholder becomes NO_VALUE, and an absent KPI value (Home savings, Purchases summary cards) no longer gets the green savings class.
• frontend/src/styles/tables.css: th.num, td.num { text-align: right; font-variant-numeric: tabular-nums }; the Purchase History and approval-queue count/money columns use it.
• frontend/src/index.html: the Purchases savings stat cards start at — instead of "$0.00".
• Tests: new __tests__/money-format.test.ts and tests-e2e/money-columns.spec.ts, new assertions in history.test.ts and dashboard.test.ts, about 40 expectations updated from --/- to —, and NO_VALUE added to 18 manual utils mocks.

Why this approach

• formatCurrency stays the single formatter instead of adding the issue's suggested formatMoney: 28 modules already import it, and en-US toLocaleString already gives the same output as Intl.NumberFormat.
• The em dash was already the majority placeholder (about 35 uses vs about 18) and the null convention in the recommendations table.
• Alignment is one shared CSS class rather than a change to every th/td, because many text columns must stay left-aligned.

Screenshots

Before is main (a14dc8f2), after is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API.

Before	After

• Opportunities, yearly period: "$123456" and "$49200", all money columns left-aligned. After: "$123,456" and "$49,200", with Count and money columns (and their headers) right-aligned with tabular digits.
• Empty Home: green --, --, bold —, green -- and -- recommendations. After: one neutral — in every tile, with no savings green on absent values.
• Purchase History: left-aligned money and - for a missing monthly cost. After: Count/Upfront/Monthly/Savings right-aligned, and the missing monthly cost shows —.

Verification

Run on this branch's head e50b553b on top of main a14dc8f2:

• Lint: 0 errors (142 pre-existing warnings). Typecheck and build pass.
• Jest: 110 suites, 3488 passed, 1 skipped. --randomize on money-format/history/utils/plans: 293 passed. A randomized run including dashboard.test.ts and recommendations.test.ts failed 14 tests, and the same 14 fail with the same seed (242556410) on untouched main, so that order dependence predates this branch. New tests: money-format.test.ts (NO_VALUE, separators in every period with the real utils), a history.test.ts assertion for which columns carry num, a dashboard.test.ts assertion that an absent savings KPI is not green, and a history banner assertion with no .value.savings.
• Playwright: 157 of 158 passed. The one failure (history-action-confirmation.spec.ts:177, a revoke refresh race) passed 14/14 when re-run alone and doesn't touch the changed code. The new money-columns.spec.ts checks computed text-align: right and tabular-nums on money cells and headers, that text columns stay left, and the yearly "$123,456".
• git-secrets scan clean (planted key flagged); test-git-secrets-allowlist.sh: 137 pass, 0 fail.

Notes for review

• savings-history.ts is untouched because chore(frontend): rename savings-history.ts's private formatCurrency to stop shadowing utils.ts's #171 and fix(frontend): purchases savings chart uses two y-axes and shadows formatCurrency #749 own that code, so the Purchases savings stat cards still format differently from the history cards until fix(frontend): purchases savings chart uses two y-axes and shadows formatCurrency #749 lands.
• Not done: a compact option on the shared formatter (it belongs with fix(frontend): purchases savings chart uses two y-axes and shadows formatCurrency #749, which uses it), and right-alignment on Inventory, Plans and RI Exchange tables. The approval dialog was already aligned.
• Worth a look: Home now shows —/mo committed and — recommendations for absent fields; right-aligned headers include their filter buttons. formatCostForPeriod(null) was already —, so the Opportunities null cells don't change.

Overlaps

• chore(frontend): rename savings-history.ts's private formatCurrency to stop shadowing utils.ts's #171 and fix(frontend): purchases savings chart uses two y-axes and shadows formatCurrency #749: savings-history.ts and the same Purchases stat cards.
• fix(frontend): monthly cost and term total never shown, bulk shows upfront 0 #711 also edits recommendations.ts and approval-details.ts, in different hunks.
• feat(frontend): de-duplicate the Purchases approval surface (Approval queue repeats Purchase History rows) #183 edits the same history test files (history-approval-queue, history-approve-button, history-cancel-button, history-cancel-permissions) and index.html: expect test-file conflicts.
• feat(frontend): Load more on the History view, Approval Queue from all loaded pages #676 also edits history.ts, tables.css, index.html and the history-truncation-notice tests.
• Any branch adding tests that expect -- from formatCurrency will conflict semantically, and other branches' history/dashboard test mocks may need NO_VALUE once this merges.

Those PRs will be opened alongside this one and merged in sequence, with conflicts resolved on the later branch.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 35 billable files and costs up to $8.75.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 6122fa2
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: bb0d1ea8-20db-423a-9c8c-a347326c9ca8

Commits

Reviewing files that changed from the base of the PR and between 6122fa2 and 412e1a4.

Files selected for processing: 35

• frontend/src/__tests__/allowed-accounts.test.ts
• frontend/src/__tests__/approval-queue-column-filters.test.ts
• frontend/src/__tests__/dashboard-ownership-950.test.ts
• frontend/src/__tests__/dashboard.test.ts
• frontend/src/__tests__/four-eyes-approval.test.ts
• frontend/src/__tests__/history-approval-queue.test.ts
• frontend/src/__tests__/history-approve-button.test.ts
• frontend/src/__tests__/history-cancel-button.test.ts
• frontend/src/__tests__/history-cancel-permissions.test.ts
• frontend/src/__tests__/history-column-filters.test.ts
• frontend/src/__tests__/history-marketplace-sell-button.test.ts
• frontend/src/__tests__/history-retry-button.test.ts
• frontend/src/__tests__/history-revoke-button.test.ts
• frontend/src/__tests__/history-truncation-notice.test.ts
• frontend/src/__tests__/history.test.ts
• frontend/src/__tests__/money-format.test.ts
• frontend/src/__tests__/plans-range-validation.test.ts
• frontend/src/__tests__/plans.test.ts
• frontend/src/__tests__/purchases-placeholders-181.test.ts
• frontend/src/__tests__/recommendations-enabled-providers.test.ts
• frontend/src/__tests__/recommendations.test.ts
• frontend/src/__tests__/trends.test.ts
• frontend/src/__tests__/utils.test.ts
• frontend/src/approval-details.ts
• frontend/src/dashboard.ts
• frontend/src/history.ts
• frontend/src/index.html
• frontend/src/plans.ts
• frontend/src/recommendations.ts
• frontend/src/styles/tables.css
• frontend/src/trends.ts
• frontend/src/utils.ts
• frontend/tests-e2e/money-columns.spec.ts
• frontend/tests-e2e/nullable-purchase-cost.spec.ts
• frontend/tests-e2e/trends.spec.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE 412e1a42c250f3709b5733140bdec108b0611af5

Blocking:

• none
  Non-blocking:
• frontend/src/history.ts:1141: the Plan cell in Purchase History still falls back to '-'. A "No recommendations attached" row now shows — in nine cells and - in Plan, which is exactly the mixed-placeholder look this PR sets out to remove. This line predates the branch and was missed. history.ts:1735-1736 (the Sell-on-Marketplace dialog rows) also still use '-'.
• frontend/src/lib/trends-data.ts:132 / frontend/src/trends.ts:226: bc7ca46 moved the Trends "Latest savings" and "Average savings" stat cards to —, but the "Coverage" card next to them still uses formatCoverage, which returns 'N/A'. The monthly table has the same mix, and trends.test.ts now locks it in with ['…', '—', 'N/A']. Either migrate it or note it as out of scope.
• frontend/src/tests/history-load-more.test.ts:17: this test came from main (feat(frontend): add Load more to History and queue from all pages #1358) through the merge. Its jest.mock('../utils') has no NO_VALUE, so in that suite MUTED_DASH and every placeholder renders the string "undefined". It passes today only because nothing there asserts on placeholders. It is the only manual utils mock not updated.
• frontend/src/styles/components.css:1231-1235: .approval-details-table th.num/td.num is now redundant with the new global th.num, td.num rule (tables.css:38-42) and can be removed.
• For the record, these placeholders remain outside the stated scope: apikeys.ts:144 '--', users/userList.ts:125 '-', inventory.ts:513/525 and ladder.ts:270-271 'N/A', approval-details.ts:253 and riexchange.ts '—' literals. The $${x.toFixed()} in modules/savings-history.ts is the only other money formatter left; the PR already defers it to fix(frontend): purchases savings chart uses two y-axes and shadows formatCurrency #749.
  Attacked: Read git diff origin/main...HEAD in full: utils, recommendations (formatScaled with PERIOD_DECIMALS), history, dashboard, plans, trends, index.html, tables.css and tests. For both merges (093719a, 202208c), diffed history.ts and recommendations.ts against main's parent. Main's escaping is kept: escapeHtml(String(p.count ?? '')), escapeHtml(formatTerm) in termCell, the providerCell whitelist, upfrontCell (fix(frontend): show the monthly charge and term total before purchase #1364), dashOr/NO_RECS_HTML (fix(frontend): stop rendering placeholder values on pending purchases #1329), and fix(frontend): Enter on a column-filter button sorts instead of opening the filter #755's aria-sort, sort-btn and filter siblings. The only changes on top of main's code are the num class and NO_VALUE. No test was deleted or loosened: the follow-ups bc7ca46 and 412e1a4 only swap --/- for —, and the num-column assertions are new. Main's newest commit (f2ab673) merges cleanly; its bulk confirm uses formatCurrency through commitmentSummary.ts, so there is no new formatter. th.sortable .sort-btn uses text-align: inherit, so headers really do right-align. Grepped the tree for --/-/N/A/toFixed/Intl.NumberFormat/currency toLocaleString. Ran 40 jest suites (money-format, history*, purchases-placeholders-181, recommendations*, dashboard, plans*, trends, utils, approval*, purchase-execution-toast, purchase-modal-fanout-payment, css*) on HEAD+main in a scratch worktree: 1519 passed, 1 skipped.

[IAmJSD]

Local verification at 412e1a42 merged onto current main (29077779; merge-tree 2b94e53e; Linux, Node, headless Chromium):

• This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
• The combined tree of every UI PR in this merge wave (tree 04af8e9b, main plus 5 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4122 passed, 4123 total), and the full Playwright suite (238 passed).
• CI is green on 412e1a42 (CI Success, Run pre-commit hooks).
• The independent review above approves this exact head.
• Nothing was applied to any cloud.

[IAmJSD]

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at 412e1a42.