fix(frontend): one money formatter and placeholder, right-align money

frontend/src/__tests__/allowed-accounts.test.ts

@@ -52,6 +52,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val: number) => `$${val || 0}`),
   formatDate: jest.fn((val: string) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years: number) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/approval-queue-column-filters.test.ts

@@ -24,6 +24,7 @@ import type { ApprovalQueueColumnFilters } from '../state';
 jest.mock('../api', () => ({}));
 jest.mock('../navigation', () => ({ switchTab: jest.fn() }));
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((v) => `$${v ?? 0}`),
   formatDate: jest.fn((v) => v),
   formatTerm: jest.fn((y) => `${y} Year${y === 1 ? '' : 's'}`),

frontend/src/__tests__/dashboard-ownership-950.test.ts

@@ -67,6 +67,7 @@ jest.mock('../state', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   getDateParts: jest.fn(() => ({ day: 15, month: 'Jan' })),
   escapeHtml: jest.fn((str) => str || ''),

frontend/src/__tests__/dashboard.test.ts

@@ -103,9 +103,10 @@ jest.mock('../state', () => ({
 
 // Mock utils
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => {
     const n = typeof val === 'number' ? val : Number(val);
-    if (val == null || !Number.isFinite(n)) return '--';
+    if (val == null || !Number.isFinite(n)) return '—';
     return `$${n}`;
   }),
   getDateParts: jest.fn(() => ({ day: 15, month: 'Jan' })),
@@ -869,11 +870,11 @@ describe('Dashboard Module', () => {
       expect(summary?.innerHTML).toContain('Active Commitments');
       expect(summary?.innerHTML).toContain('Current Coverage');
       expect(summary?.innerHTML).toContain('YTD Savings');
-      // Savings card must fall back to '--' (not throw, go blank, or fabricate $0).
+      // Savings card must fall back to '—' (not throw, go blank, or fabricate $0).
       const savingsCard = summary?.querySelector('.card');
       expect(savingsCard?.textContent).toContain('Potential Monthly Savings');
-      // When recs fail cellCount=0, the sentinel '--' is shown instead of fabricated $0.
-      expect(savingsCard?.innerHTML).toContain('--');
+      // When recs fail cellCount=0, the sentinel '—' is shown instead of fabricated $0.
+      expect(savingsCard?.innerHTML).toContain('—');
       expect(savingsCard?.innerHTML).not.toContain('$0');
     });
 
@@ -934,10 +935,10 @@ describe('Dashboard Module', () => {
       expect(summary?.innerHTML).toContain('Current Coverage');
       expect(summary?.innerHTML).toContain('YTD Savings');
 
-      // Savings card shows '--' when recs are absent (no fabricated $0).
+      // Savings card shows '—' when recs are absent (no fabricated $0).
       const savingsCard = summary?.querySelector('.card');
       expect(savingsCard?.textContent).toContain('Potential Monthly Savings');
-      expect(savingsCard?.innerHTML).toContain('--');
+      expect(savingsCard?.innerHTML).toContain('—');
       expect(savingsCard?.innerHTML).not.toContain('$0');
     });
 
@@ -963,9 +964,9 @@ describe('Dashboard Module', () => {
       const summary = document.getElementById('summary');
       expect(summary?.innerHTML).toContain('Active Commitments');
 
-      // Savings card shows '--' (no fabricated $0 when recs are absent).
+      // Savings card shows '—' (no fabricated $0 when recs are absent).
       const savingsCard = summary?.querySelector('.card');
-      expect(savingsCard?.innerHTML).toContain('--');
+      expect(savingsCard?.innerHTML).toContain('—');
       expect(savingsCard?.innerHTML).not.toContain('$0');
     });
 
@@ -2146,12 +2147,12 @@ describe('Dashboard Module', () => {
     });
   });
 
-  // H-3 regression: absent API coverage/count fields must render '--', not '$0'/'80%'/'0'.
+  // H-3 regression: absent API coverage/count fields must render '—', not '$0'/'80%'/'0'.
   // Pre-fix: `data.current_coverage || 0` fabricated '0%'; `data.target_coverage || 80`
   // fabricated '80%'; `data.active_commitments || 0` fabricated '0'; the savings catch
   // block fell back to formatCurrency(0). These tests must FAIL on pre-fix code.
-  describe('H-3: absent dashboard KPI fields render -- sentinel, not fabricated values', () => {
-    test('absent target_coverage shows -- not hardcoded 80%', async () => {
+  describe('H-3: absent dashboard KPI fields render the em-dash sentinel, not fabricated values', () => {
+    test('absent target_coverage shows an em dash, not hardcoded 80%', async () => {
       (api.getDashboardSummary as jest.Mock).mockResolvedValue({
         // target_coverage deliberately absent
         total_recommendations: 1,
@@ -2169,11 +2170,11 @@ describe('Dashboard Module', () => {
       // Must not show the hardcoded 80% fallback
       expect(summary?.textContent).not.toContain('Target: 80.0%');
       expect(summary?.textContent).not.toContain('Target: 80%');
-      // Must show the '--' sentinel for absent target
-      expect(summary?.textContent).toContain('Target: --');
+      // Must show the '—' sentinel for absent target
+      expect(summary?.textContent).toContain('Target: —');
     });
 
-    test('absent current_coverage shows -- not fabricated 0%', async () => {
+    test('absent current_coverage shows an em dash, not fabricated 0%', async () => {
       (api.getDashboardSummary as jest.Mock).mockResolvedValue({
         // current_coverage deliberately absent
         total_recommendations: 2,
@@ -2195,11 +2196,11 @@ describe('Dashboard Module', () => {
       );
       expect(coverageTile).toBeDefined();
       const valueEl = coverageTile?.querySelector('.kpi-tile-value');
-      // Must show '--' (not a fabricated '0.0%') when current_coverage is absent
-      expect(valueEl?.textContent).toBe('--');
+      // Must show the em dash (not a fabricated '0.0%') when current_coverage is absent
+      expect(valueEl?.textContent).toBe('—');
     });
 
-    test('absent active_commitments shows -- not fabricated 0', async () => {
+    test('absent active_commitments shows an em dash, not fabricated 0', async () => {
       (api.getDashboardSummary as jest.Mock).mockResolvedValue({
         total_recommendations: 1,
         // active_commitments deliberately absent
@@ -2220,12 +2221,12 @@ describe('Dashboard Module', () => {
         t.textContent?.includes('Active Commitments')
       );
       expect(commitmentsTile).toBeDefined();
-      // Must not show fabricated '0'; must show '--'
+      // Must not show fabricated '0'; must show '—'
       const valueEl = commitmentsTile?.querySelector('.kpi-tile-value');
-      expect(valueEl?.textContent).toBe('--');
+      expect(valueEl?.textContent).toBe('—');
     });
 
-    test('savings range catch block shows -- not $0 when computation throws', async () => {
+    test('savings range catch block shows an em dash, not $0 when computation throws', async () => {
       // Force the groupRecsByCell mock to throw so the catch block fires.
       mockGroupRecsByCell.mockImplementationOnce(() => { throw new Error('compute error'); });
       (api.getDashboardSummary as jest.Mock).mockResolvedValue({
@@ -2248,8 +2249,10 @@ describe('Dashboard Module', () => {
       );
       expect(savingsTile).toBeDefined();
       const valueEl = savingsTile?.querySelector('.kpi-tile-value');
-      // Must show '--' (not '$0') when the savings computation fails
-      expect(valueEl?.textContent).toBe('--');
+      // Must show the em dash (not '$0') when the savings computation fails,
+      // and not in savings green: it is not a saving (issue #739).
+      expect(valueEl?.textContent).toBe('—');
+      expect(valueEl?.classList.contains('savings')).toBe(false);
     });
   });
 });

frontend/src/__tests__/four-eyes-approval.test.ts

@@ -22,6 +22,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-approval-queue.test.ts

@@ -41,6 +41,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-approve-button.test.ts

@@ -35,6 +35,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-cancel-button.test.ts

@@ -29,6 +29,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-cancel-permissions.test.ts

@@ -31,6 +31,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-column-filters.test.ts

@@ -28,6 +28,7 @@ import type { PurchaseHistoryColumnFilters } from '../state';
 jest.mock('../api', () => ({}));
 jest.mock('../navigation', () => ({ switchTab: jest.fn() }));
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((v) => `$${v ?? 0}`),
   formatDate: jest.fn((v) => v),
   formatTerm: jest.fn((y) => `${y} Year${y === 1 ? '' : 's'}`),

frontend/src/__tests__/history-marketplace-sell-button.test.ts

@@ -38,6 +38,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-retry-button.test.ts

@@ -38,6 +38,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-revoke-button.test.ts

@@ -36,6 +36,7 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history-truncation-notice.test.ts

@@ -13,6 +13,7 @@ jest.mock('../api', () => ({
 }));
 jest.mock('../navigation', () => ({ switchTab: jest.fn() }));
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
   formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),

frontend/src/__tests__/history.test.ts

@@ -21,8 +21,9 @@ jest.mock('../permissions', () => ({
 }));
 
 jest.mock('../utils', () => ({
-  // Mirrors the real formatCurrency behaviour: null/undefined/NaN -> '--', numbers -> '$<val>'
-  formatCurrency: jest.fn((val) => (val === null || val === undefined || isNaN(val)) ? '--' : `$${val}`),
+  NO_VALUE: '\u2014',
+  // Mirrors the real formatCurrency behaviour: null/undefined/NaN -> '—', numbers -> '$<val>'
+  formatCurrency: jest.fn((val) => (val === null || val === undefined || isNaN(val)) ? '—' : `$${val}`),
   formatDate: jest.fn((val) => val ? new Date(val).toLocaleDateString() : ''),
   formatTerm: jest.fn((years) => years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`),
   escapeHtml: jest.fn((str) => str || ''),
@@ -100,10 +101,15 @@ describe('History Module', () => {
     await loadHistory();
     const history = document.querySelector('#history-list tr[data-execution-id="complete"]');
     const queue = document.querySelector('#purchases-approval-queue tr[data-execution-id="pending"]');
-    expect(history?.querySelectorAll('td')[8]?.textContent).toBe('--');
-    expect(history?.querySelectorAll('td')[9]?.textContent).toBe(amortize ? '-' : '$50');
-    expect(queue?.querySelectorAll('td')[7]?.textContent).toBe(amortize ? '-' : '$50');
-    expect(queue?.querySelectorAll('td')[8]?.textContent).toBe('--');
+    expect(history?.querySelectorAll('td')[8]?.textContent).toBe('—');
+    expect(history?.querySelectorAll('td')[9]?.textContent).toBe(amortize ? '—' : '$50');
+    expect(queue?.querySelectorAll('td')[7]?.textContent).toBe(amortize ? '—' : '$50');
+    expect(queue?.querySelectorAll('td')[8]?.textContent).toBe('—');
+    // Count and money columns right-align (issue #739).
+    const numCols = (row: Element | null): number[] => Array.from(row?.querySelectorAll('td') ?? [])
+      .flatMap((td, i) => (td.classList.contains('num') ? [i] : []));
+    expect(numCols(history)).toEqual([6, 8, 9, 10]);
+    expect(numCols(queue)).toEqual([4, 7, 8, 9]);
   });
 
   // Issue #318: with amortize on, the upfront is already in Monthly Cost, so
@@ -674,12 +680,12 @@ describe('History Module', () => {
     });
   });
 
-  // H-2 regression: absent API summary must render '--' on KPI cards, not '$0'.
+  // H-2 regression: absent API summary must render '—' on KPI cards, not '$0'.
   // Pre-fix: `data.summary || {}` passed an empty HistorySummary to
   // renderHistorySummary, and `?? 0` coercions made money fields render as
   // '$0' (or the mock's '$0'). These tests must FAIL on the pre-fix code.
-  describe('H-2: absent summary renders -- sentinel on KPI cards', () => {
-    test('renderHistorySummary: absent summary (undefined) shows -- on all money cards (loadHistory)', async () => {
+  describe('H-2: absent summary renders the em-dash sentinel on KPI cards', () => {
+    test('renderHistorySummary: absent summary (undefined) shows an em dash on all money cards (loadHistory)', async () => {
       (api.getHistory as jest.Mock).mockResolvedValue({
         // summary field deliberately absent
         purchases: []
@@ -692,14 +698,14 @@ describe('History Module', () => {
       expect(summary?.innerHTML).toContain('Monthly Savings');
       expect(summary?.innerHTML).toContain('Annual Savings');
       // Must show the absent sentinel, not a fabricated '$0'
-      expect(summary?.innerHTML).toContain('--');
-      // The mock formatCurrency returns '$0' for 0 and '--' for null;
+      expect(summary?.innerHTML).toContain('—');
+      // The mock formatCurrency returns '$0' for 0 and an em dash for null;
       // verify no '$0' slips through for the money cards.
       expect(summary?.textContent).not.toMatch(/Total Upfront Spent.*\$0/s);
       expect(summary?.textContent).not.toMatch(/Monthly Savings.*\$0/s);
     });
 
-    test('renderHistorySummary: absent total_upfront renders -- not $0 (viewPlanHistory)', async () => {
+    test('renderHistorySummary: absent total_upfront renders an em dash, not $0 (viewPlanHistory)', async () => {
       (api.getHistory as jest.Mock).mockResolvedValue({
         summary: {
           total_purchases: 3,
@@ -713,16 +719,16 @@ describe('History Module', () => {
       await viewPlanHistory('plan-abc');
 
       const summary = document.getElementById('history-summary');
-      // total_purchases is present; money fields that are absent must be '--'
+      // total_purchases is present; money fields that are absent must be an em dash
       expect(summary?.textContent).toContain('3');
       // formatCurrency must be called with null for the absent total_upfront
-      // field and must produce '--', not '$0'. This assertion fails if the
+      // field and must produce an em dash, not '$0'. This assertion fails if the
       // production code regresses to passing 0 or a fabricated value.
       const { formatCurrency } = jest.requireMock('../utils') as { formatCurrency: jest.Mock };
       expect(formatCurrency).toHaveBeenCalledWith(null);
-      // The rendered card must show '--', not a dollar amount.
+      // The rendered card must show an em dash, not a dollar amount.
       expect(summary?.innerHTML).toContain('Total Upfront Spent');
-      expect(summary?.innerHTML).toContain('<p class="value">--</p>');
+      expect(summary?.innerHTML).toContain('<p class="value">—</p>');
       expect(summary?.innerHTML).not.toContain('<p class="value">$0</p>');
     });
 
@@ -771,14 +777,16 @@ describe('History Module', () => {
       await loadHistory();
 
       const summary = document.getElementById('history-summary');
-      // All four cards must still render (just with '--' values).
+      // All four cards must still render (just with em-dash values).
       expect(summary?.innerHTML).toContain('Total Purchases');
       expect(summary?.innerHTML).toContain('Total Upfront Spent');
       expect(summary?.innerHTML).toContain('Monthly Savings');
       expect(summary?.innerHTML).toContain('Annual Savings');
-      // The unknown-state banner always shows '--' for every value.
-      const allDashes = (summary?.innerHTML.match(/--/g) || []).length;
+      // The unknown-state banner shows the em dash for every value, and an
+      // absent figure is not styled as a saving (issue #739).
+      const allDashes = (summary?.innerHTML.match(/—/g) || []).length;
       expect(allDashes).toBeGreaterThanOrEqual(4);
+      expect(summary?.querySelector('.value.savings')).toBeNull();
     });
   });
 });

frontend/src/__tests__/money-format.test.ts

@@ -0,0 +1,23 @@
+/**
+ * Issue #739: money goes through one formatter. The period-scaled cost
+ * cells used to print `$${x.toFixed(n)}`, so a yearly figure read
+ * "$123456" with no separators, and absent values used three different
+ * placeholders.
+ */
+import { formatCostForPeriod } from '../recommendations';
+import { formatCurrency, NO_VALUE } from '../utils';
+
+describe('money formatting (issue #739)', () => {
+  it('uses one em-dash placeholder for absent values', () => {
+    expect(NO_VALUE).toBe('—');
+    expect(formatCurrency(null)).toBe(NO_VALUE);
+    expect(formatCostForPeriod(null, 'yearly')).toBe(NO_VALUE);
+  });
+
+  it('groups thousands in every cost period', () => {
+    expect(formatCostForPeriod(10288, 'yearly')).toBe('$123,456');
+    expect(formatCostForPeriod(100000, 'daily')).toBe('$3,333.33');
+    expect(formatCostForPeriod(2000000, 'hourly')).toBe('$2,777.7778');
+    expect(formatCostForPeriod(1234.4, 'monthly')).toBe('$1,234');
+  });
+});

frontend/src/__tests__/plans-range-validation.test.ts

@@ -72,6 +72,7 @@ jest.mock('../confirmDialog', () => ({
 }));
 
 jest.mock('../utils', () => ({
+  NO_VALUE: '\u2014',
   formatDate: jest.fn((v) => v ?? ''),
   formatTerm: jest.fn((y) => `${y}yr`),
   formatRampSchedule: jest.fn((v) => v ?? ''),