From 003170d75a236d0c2bcbe98003d881cfbc512f9f Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 23:05:34 +0000 Subject: [PATCH 1/3] feat(frontend): add formatCompactCurrency for chart axes Chart axes need short labels ("$1.5K", "$12.35K", "$1.23M"). The savings chart built them by hand with `$${(v / 1000).toFixed(1)}K`. Add one shared helper on Intl.NumberFormat's compact notation, pinned to en-US like formatCurrency and with the same absent-value marker, so the chart can drop its own formatting (issue #749). Refs #749 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/utils.test.ts | 21 +++++++++++++++++++++ frontend/src/utils.ts | 18 ++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/frontend/src/__tests__/utils.test.ts b/frontend/src/__tests__/utils.test.ts index 58b4759c9..2cfc8976c 100644 --- a/frontend/src/__tests__/utils.test.ts +++ b/frontend/src/__tests__/utils.test.ts @@ -2,6 +2,7 @@ * Unit tests for utility functions */ import { + formatCompactCurrency, formatCurrency, formatDate, formatDateTime, @@ -640,6 +641,26 @@ describe('jsonClone (explicit JSON-serialisation variant)', () => { // --------------------------------------------------------------------------- // Regression tests for finding 11-N2: formatCurrency distinguishes absent from $0. // --------------------------------------------------------------------------- +describe('formatCompactCurrency (issue #749)', () => { + test('abbreviates thousands and millions like a chart axis', () => { + expect(formatCompactCurrency(0)).toBe('$0'); + expect(formatCompactCurrency(950)).toBe('$950'); + expect(formatCompactCurrency(1500)).toBe('$1.5K'); + expect(formatCompactCurrency(12345)).toBe('$12.35K'); + expect(formatCompactCurrency(1234567)).toBe('$1.23M'); + expect(formatCompactCurrency(12345, 1)).toBe('$12.3K'); + }); + + test('keeps close fractional ticks distinct', () => { + expect(formatCompactCurrency(0.1)).not.toBe(formatCompactCurrency(0.2)); + }); + + test('uses the same absent marker as formatCurrency', () => { + expect(formatCompactCurrency(null)).toBe(formatCurrency(null)); + expect(formatCompactCurrency(NaN)).toBe(formatCurrency(NaN)); + }); +}); + describe('formatCurrency (11-N2: absent vs real zero)', () => { test('null returns -- not $0', () => { expect(formatCurrency(null as unknown as number)).toBe('--'); diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts index ab2b3eeae..d6545c5cc 100644 --- a/frontend/src/utils.ts +++ b/frontend/src/utils.ts @@ -54,6 +54,24 @@ export function formatCurrency( })}`; } +/** + * Format a number as compact currency ("$950", "$1.5K", "$12.35K", "$3M") + * for chart axes, where full figures would crowd the ticks. Same en-US + * pinning and absent-value marker as formatCurrency; `digits` caps the + * fraction digits so close ticks (0.1 vs 0.2) stay distinct. + */ +export function formatCompactCurrency(value: number | null | undefined, digits = 2): string { + if (value === null || value === undefined || !Number.isFinite(value)) { + return '--'; + } + return new Intl.NumberFormat('en-US', { + style: 'currency', + currency: 'USD', + notation: 'compact', + maximumFractionDigits: digits, + }).format(value); +} + /** * Parse a date input into a Date, treating bare calendar dates correctly. * From 43d1d5fd907a08291b5e20154cbafda943bc3244 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 23:05:34 +0000 Subject: [PATCH 2/3] fix(frontend): plot one savings series on one axis with shared formatters The Purchases savings chart drew period savings (left axis) and cumulative savings (right axis) in one plot, so the points where the lines crossed meant nothing. Its ticks read "$1500.00", and the module had its own formatCurrency ("$1.50K") with the same name as the shared helper in utils.ts, so the stat cards formatted money differently from the rest of the Purchases page. - Period savings are bars on a single y-axis with compact ticks (formatCompactCurrency: "$1.5K"). The running total is a separate view, chosen with a new Show: Savings per period / Cumulative savings select (#savings-series), on the same single axis. - Tooltips and the Period / Avg / Peak stat cards use the shared formatCurrency with cents ("$12,840.00", "$428.00/mo"); the local formatCurrency is removed. - The x-axis keeps its period-start anchor (QA 2.2) with the bar chart's default offset turned off. Refs #749 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/savings-history.test.ts | 166 ++++++++-------- frontend/src/index.html | 6 + frontend/src/modules/savings-history.ts | 180 ++++++++---------- .../tests-e2e/savings-history-average.spec.ts | 14 +- 4 files changed, 174 insertions(+), 192 deletions(-) diff --git a/frontend/src/__tests__/savings-history.test.ts b/frontend/src/__tests__/savings-history.test.ts index df1b1bb9f..663249b79 100644 --- a/frontend/src/__tests__/savings-history.test.ts +++ b/frontend/src/__tests__/savings-history.test.ts @@ -56,6 +56,10 @@ describe('Savings History Module', () => { +
@@ -293,7 +297,7 @@ describe('Savings History Module', () => { const avgSavingsEl = document.getElementById('avg-hourly-savings'); const peakSavingsEl = document.getElementById('peak-savings'); - expect(periodSavingsEl?.textContent).toContain('$1.50K'); + expect(periodSavingsEl?.textContent).toContain('$1,500.00'); expect(avgSavingsEl?.textContent).toContain('$75.00/mo'); expect(peakSavingsEl?.textContent).toContain('$200.00/mo'); }); @@ -337,7 +341,7 @@ describe('Savings History Module', () => { expect(mockChartInstance.destroy).toHaveBeenCalled(); }); - test('formats currency values over 1000 with K suffix', async () => { + test('formats currency values over 1000 with the shared formatter (#749)', async () => { const mockData = { summary: { total_period_savings: 5000, @@ -353,7 +357,7 @@ describe('Savings History Module', () => { await loadSavingsHistory(); const periodSavingsEl = document.getElementById('period-savings'); - expect(periodSavingsEl?.textContent).toBe('$5.00K'); + expect(periodSavingsEl?.textContent).toBe('$5,000.00'); }); test('formats currency values under 1000 without suffix', async () => { @@ -428,19 +432,18 @@ describe('Savings History Module', () => { await loadSavingsHistory(); + // Issue #749: one series (period savings as bars) on one y-axis. expect(Chart).toHaveBeenCalledWith( expect.any(HTMLCanvasElement), expect.objectContaining({ - type: 'line', + type: 'bar', data: expect.objectContaining({ - datasets: expect.arrayContaining([ + datasets: [ expect.objectContaining({ + type: 'bar', label: 'Period Savings' - }), - expect.objectContaining({ - label: 'Cumulative Savings' }) - ]) + ] }), options: expect.objectContaining({ responsive: true, @@ -746,6 +749,7 @@ describe('Savings History Module', () => { (getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: manyDataPoints }); + (document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative'; await loadSavingsHistory(); @@ -764,6 +768,7 @@ describe('Savings History Module', () => { (getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: fewDataPoints }); + (document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative'; await loadSavingsHistory(); @@ -795,9 +800,11 @@ describe('Savings History Module', () => { const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; const yAxisCallback = chartCall[1].options.scales.y.ticks.callback; - // Test with number value - expect(yAxisCallback(50)).toBe('$50.00'); + // Compact shared formatter, not `$${x.toFixed(2)}` (issue #749) + expect(yAxisCallback(50)).toBe('$50'); expect(yAxisCallback(123.456)).toBe('$123.46'); + expect(yAxisCallback(1500)).toBe('$1.5K'); + expect(yAxisCallback(12500)).toBe('$12.5K'); }); test('y-axis tick callback formats currency for string values', async () => { @@ -814,64 +821,9 @@ describe('Savings History Module', () => { const yAxisCallback = chartCall[1].options.scales.y.ticks.callback; // Test with string value - expect(yAxisCallback('75.5')).toBe('$75.50'); - expect(yAxisCallback('100')).toBe('$100.00'); - }); - - test('y1-axis tick callback formats large values with K suffix', async () => { - const mockData = { - data_points: [ - { timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 } - ] - }; - (getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData); - - await loadSavingsHistory(); - - const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; - const y1AxisCallback = chartCall[1].options.scales.y1.ticks.callback; - - // Test with large number values (>= 1000) - expect(y1AxisCallback(1000)).toBe('$1.0K'); - expect(y1AxisCallback(5000)).toBe('$5.0K'); - expect(y1AxisCallback(12500)).toBe('$12.5K'); - }); - - test('y1-axis tick callback formats small values without K suffix', async () => { - const mockData = { - data_points: [ - { timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 } - ] - }; - (getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData); - - await loadSavingsHistory(); - - const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; - const y1AxisCallback = chartCall[1].options.scales.y1.ticks.callback; - - // Test with small number values (< 1000) - expect(y1AxisCallback(50)).toBe('$50'); - expect(y1AxisCallback(999)).toBe('$999'); - expect(y1AxisCallback(0)).toBe('$0'); - }); - - test('y1-axis tick callback handles string values', async () => { - const mockData = { - data_points: [ - { timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 } - ] - }; - (getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData); - - await loadSavingsHistory(); - - const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; - const y1AxisCallback = chartCall[1].options.scales.y1.ticks.callback; - - // Test with string values - expect(y1AxisCallback('2500')).toBe('$2.5K'); - expect(y1AxisCallback('500')).toBe('$500'); + expect(yAxisCallback('75.5')).toBe('$75.5'); + expect(yAxisCallback('100')).toBe('$100'); + expect(yAxisCallback('2500')).toBe('$2.5K'); }); test('tooltip label callback formats period savings with unit suffix (default monthly)', async () => { @@ -903,19 +855,19 @@ describe('Savings History Module', () => { ] }; (getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData); + (document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative'; await loadSavingsHistory(); const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; const tooltipLabelCallback = chartCall[1].options.plugins.tooltip.callbacks.label; - // Test cumulative savings (datasetIndex 1) const cumulativeContext = { raw: 1234.56, - datasetIndex: 1, + datasetIndex: 0, dataset: { label: 'Cumulative Savings' } }; - expect(tooltipLabelCallback(cumulativeContext)).toBe('Cumulative Savings: $1234.56'); + expect(tooltipLabelCallback(cumulativeContext)).toBe('Cumulative Savings: $1,234.56'); }); test('tooltip label callback handles null/undefined raw value', async () => { @@ -941,10 +893,10 @@ describe('Savings History Module', () => { const undefinedContext = { raw: undefined, - datasetIndex: 1, - dataset: { label: 'Cumulative Savings' } + datasetIndex: 0, + dataset: { label: 'Period Savings' } }; - expect(tooltipLabelCallback(undefinedContext)).toBe('Cumulative Savings: $0.00'); + expect(tooltipLabelCallback(undefinedContext)).toBe('Period Savings: $0.00/mo'); }); // Regression tests for QA 2.2 / 2.3 / 2.4 (do not remove) @@ -1012,7 +964,7 @@ describe('Savings History Module', () => { expect(chartCall[1].data.labels).toBeUndefined(); }); - test('QA 2.4: y and y1 axes both have maxTicksLimit to prevent tick instability', async () => { + test('QA 2.4: the y-axis has maxTicksLimit to prevent tick instability', async () => { const mockData = { data_points: [ { timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 } @@ -1024,15 +976,12 @@ describe('Savings History Module', () => { const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; const yTicks = chartCall[1].options.scales.y.ticks; - const y1Ticks = chartCall[1].options.scales.y1.ticks; expect(typeof yTicks.maxTicksLimit).toBe('number'); expect(yTicks.maxTicksLimit).toBeGreaterThan(0); - expect(typeof y1Ticks.maxTicksLimit).toBe('number'); - expect(y1Ticks.maxTicksLimit).toBeGreaterThan(0); }); - test('QA 2.5: y1-axis formatter does not collapse distinct floats to the same integer label', async () => { + test('QA 2.5: the y-axis formatter does not collapse distinct floats to the same integer label', async () => { const mockData = { data_points: [ { timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 } @@ -1043,7 +992,7 @@ describe('Savings History Module', () => { await loadSavingsHistory(); const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; - const y1Cb = chartCall[1].options.scales.y1.ticks.callback; + const y1Cb = chartCall[1].options.scales.y.ticks.callback; // Two floats that differ in decimal but share the same integer part must // produce distinct labels (QA 2.5). @@ -1403,8 +1352,8 @@ describe('Savings History Module', () => { const avgEl = document.getElementById('avg-hourly-savings'); expect(avgEl?.textContent).toContain('/yr'); - // 730 * 12 = $8760.00 -> formatted as $8.76K - expect(avgEl?.textContent).toContain('$8.76K'); + // 730 * 12 = $8,760.00 + expect(avgEl?.textContent).toContain('$8,760.00'); }); test('avg-savings-label heading changes to reflect selected unit', async () => { @@ -1469,10 +1418,10 @@ describe('Savings History Module', () => { expect(document.getElementById('period-savings')?.textContent).toBe('$1.00'); expect(document.getElementById('period-savings-unit')?.textContent).toContain('hourly'); - // Yearly: 730 * 12 = 8760 -> $8.76K; sub-line updates to 'yearly' + // Yearly: 730 * 12 = 8760 -> $8,760.00; sub-line updates to 'yearly' unitSelect.value = 'yearly'; await loadSavingsHistory(); - expect(document.getElementById('period-savings')?.textContent).toBe('$8.76K'); + expect(document.getElementById('period-savings')?.textContent).toBe('$8,760.00'); expect(document.getElementById('period-savings-unit')?.textContent).toContain('yearly'); }); @@ -1488,4 +1437,51 @@ describe('Savings History Module', () => { expect(tooltipCb(ctx)).toContain('/hr'); }); }); + + describe('one series on one axis (issue #749)', () => { + const mockData = { + data_points: [ + { timestamp: '2024-01-01T00:00:00Z', total_savings: 100, cumulative_savings: 100, total_upfront: 0, purchase_count: 1 }, + { timestamp: '2024-01-02T00:00:00Z', total_savings: 300, cumulative_savings: 400, total_upfront: 0, purchase_count: 1 }, + ], + }; + + test('period view draws bars on a single y-axis', async () => { + (getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData); + await loadSavingsHistory(); + const config = (Chart as unknown as jest.Mock).mock.calls[0][1]; + expect(Object.keys(config.options.scales)).toEqual(['x', 'y']); + expect(config.data.datasets).toHaveLength(1); + expect(config.data.datasets[0].type).toBe('bar'); + expect(config.data.datasets[0].data.map((d: { y: number }) => d.y)).toEqual([100, 300]); + expect(config.options.scales.y.title.text).toBe('Savings per period (Monthly)'); + }); + + test('cumulative view plots the running total alone, without a rate suffix', async () => { + (getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData); + (document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative'; + await loadSavingsHistory(); + const config = (Chart as unknown as jest.Mock).mock.calls[0][1]; + expect(Object.keys(config.options.scales)).toEqual(['x', 'y']); + expect(config.type).toBe('line'); + expect(config.data.datasets).toHaveLength(1); + expect(config.data.datasets[0].label).toBe('Cumulative Savings'); + expect(config.data.datasets[0].data.map((d: { y: number }) => d.y)).toEqual([100, 400]); + expect(config.options.scales.y.title.text).toBe('Cumulative savings'); + const tooltip = config.options.plugins.tooltip.callbacks.label; + expect(tooltip({ raw: { x: 0, y: 400 }, dataset: { label: 'Cumulative Savings' } })).toBe('Cumulative Savings: $400.00'); + }); + + test('changing the series reloads the chart once, even after a second init', async () => { + (getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData); + initSavingsHistory(); + initSavingsHistory(); + (getSavingsAnalytics as jest.Mock).mockClear(); + const series = document.getElementById('savings-series') as HTMLSelectElement; + series.value = 'cumulative'; + series.dispatchEvent(new Event('change')); + await new Promise(resolve => setTimeout(resolve, 50)); + expect(getSavingsAnalytics).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/frontend/src/index.html b/frontend/src/index.html index ebbbdd24c..213bfa6a2 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -204,6 +204,12 @@

Savings History

+
diff --git a/frontend/src/modules/savings-history.ts b/frontend/src/modules/savings-history.ts index 60a4332f5..2b671f7b1 100644 --- a/frontend/src/modules/savings-history.ts +++ b/frontend/src/modules/savings-history.ts @@ -6,6 +6,7 @@ import { Chart, registerables } from 'chart.js'; import { getSavingsAnalytics, type SavingsAnalyticsResponse, type SavingsDataPoint } from '../api'; import * as state from '../state'; import { formatTrendAxisTick } from './chart-utils'; +import { formatCompactCurrency, formatCurrency } from '../utils'; // Register Chart.js components Chart.register(...registerables); @@ -71,6 +72,18 @@ function getSelectedUnit(): SavingsUnit { return API_UNIT; } +/** + * Which series the chart plots (issue #749). Period savings and the running + * total have different scales and units, so they are shown one at a time on + * a single axis instead of on two axes whose line crossings mean nothing. + */ +export type SavingsSeries = 'period' | 'cumulative'; + +function getSelectedSeries(): SavingsSeries { + const el = document.getElementById('savings-series') as HTMLSelectElement | null; + return el?.value === 'cumulative' ? 'cumulative' : 'period'; +} + /** * Load savings history data based on selected period */ @@ -119,7 +132,7 @@ export async function loadSavingsHistory(): Promise { if (statsEl) statsEl.classList.remove('hidden'); renderSavingsStats(data, start, end, interval); - renderSavingsChart(data.data_points, interval, getSelectedUnit(), start, end); + renderSavingsChart(data.data_points, interval, getSelectedUnit(), start, end, getSelectedSeries()); } catch (error) { const msg = error instanceof Error ? error.message : 'Unknown error'; console.error('Failed to load savings history:', msg); @@ -305,7 +318,7 @@ function renderSavingsStats(data: SavingsAnalyticsResponse, start: Date, end: Da if (periodSavingsEl) { // Period Savings is the cumulative total over the selected date range // (no per-unit rate suffix -- it is already a dollar total). - periodSavingsEl.textContent = formatCurrency(displayTotal); + periodSavingsEl.textContent = formatCurrency(displayTotal, '$', 2); } if (periodSavingsLabelEl) { periodSavingsLabelEl.textContent = 'Period Savings'; @@ -317,44 +330,30 @@ function renderSavingsStats(data: SavingsAnalyticsResponse, start: Date, end: Da avgLabelEl.textContent = `Avg ${adjective} Savings`; } if (avgSavingsEl) { - avgSavingsEl.textContent = `${formatCurrency(displayAvg)}${suffix}`; + avgSavingsEl.textContent = `${formatCurrency(displayAvg, '$', 2)}${suffix}`; } if (peakSavingsEl) { - peakSavingsEl.textContent = `${formatCurrency(displayPeak)}${suffix}`; - } -} - -/** - * Format currency value for chart contexts (abbreviated K for >= $1000). - * - * M-7: add a null/non-finite guard so a future API change that introduces - * null on total_savings or cumulative_savings doesn't crash the chart with - * a TypeError on .toFixed(). Returns '--' for absent/non-finite values, - * matching the shared formatCurrency sentinel in utils.ts. - */ -function formatCurrency(value: number | null | undefined): string { - if (value == null || !Number.isFinite(value)) { - return '--'; - } - if (value >= 1000) { - return `$${(value / 1000).toFixed(2)}K`; + peakSavingsEl.textContent = `${formatCurrency(displayPeak, '$', 2)}${suffix}`; } - return `$${value.toFixed(2)}`; } /** * Render savings chart using Chart.js. * - * BUG FIX (QA 2.2): the x-axis is now `type:'linear'` anchored to the selected + * Issue #749: one series on one y-axis. Period savings are bars (each bar is + * one interval's projected monthly additions, converted to the chosen unit); + * the running total is a separate view picked with #savings-series. The old + * chart drew both on two y-axes, so the points where the lines crossed had + * no meaning. Ticks and tooltips use the shared formatters ("$1.5K" on the + * axis, "$1,500.00" in the tooltip) instead of `$${x.toFixed(2)}`. + * + * BUG FIX (QA 2.2): the x-axis is `type:'linear'` anchored to the selected * period [start, end] so the leftmost tick is the period start, not the first * data point. Mirrors the approach used by the Home dashboard chart (PR #746). * - * BUG FIX (QA 2.3): Period Savings tooltip now uses toFixed(2) matching - * Cumulative and the KPI box above the chart. - * - * BUG FIX (QA 2.4/2.5): Both y-axes have maxTicksLimit:6 to prevent tick - * instability when toggling series; y1 formatter avoids duplicate integer - * labels by showing 2 decimal places for non-integer float ticks. + * BUG FIX (QA 2.4/2.5): the y-axis has maxTicksLimit:6 to prevent tick + * instability; the compact formatter keeps up to two fraction digits so + * close float ticks stay distinct. */ function renderSavingsChart( dataPoints: SavingsDataPoint[], @@ -362,6 +361,7 @@ function renderSavingsChart( unit: SavingsUnit = 'monthly', periodStart?: Date, periodEnd?: Date, + series: SavingsSeries = 'period', ): void { const ctx = document.getElementById('savings-history-chart') as HTMLCanvasElement; @@ -370,7 +370,10 @@ function renderSavingsChart( return; } - const suffix = unitSuffix(unit); + const cumulative = series === 'cumulative'; + // The running total is a dollar amount, not a rate, so it takes no + // unit suffix (its unit conversion is tracked separately in #134). + const suffix = cumulative ? '' : unitSuffix(unit); // Determine the interval hint for the x-axis tick formatter. // The interval parameter can be 'hourly', 'daily', 'weekly', or 'monthly'; @@ -382,13 +385,9 @@ function renderSavingsChart( // Map each data point to {x: timestamp_ms, y: value} so Chart.js positions // each point at its real date on the linear axis instead of by label index. // M-6: use ?? 0 (not || 0) to treat a genuine 0-saving bucket correctly. - const periodSavingsData = dataPoints.map(dp => ({ + const points = dataPoints.map(dp => ({ x: new Date(dp.timestamp).getTime(), - y: convertFromMonthly(dp.total_savings ?? 0, unit), - })); - const cumulativeSavingsData = dataPoints.map(dp => ({ - x: new Date(dp.timestamp).getTime(), - y: dp.cumulative_savings ?? 0, + y: cumulative ? dp.cumulative_savings ?? 0 : convertFromMonthly(dp.total_savings ?? 0, unit), })); // Axis bounds: anchor to the selected period so the leftmost tick is the @@ -401,34 +400,33 @@ function renderSavingsChart( savingsChart.destroy(); } + const label = cumulative ? 'Cumulative Savings' : 'Period Savings'; + const dataset = cumulative + ? { + type: 'line' as const, + label, + data: points, + borderColor: '#4285f4', + backgroundColor: 'rgba(66, 133, 244, 0.1)', + fill: true, + tension: 0.3, + pointRadius: dataPoints.length > 50 ? 0 : 3, + pointHoverRadius: 5, + } + : { + type: 'bar' as const, + label, + data: points, + backgroundColor: 'rgba(52, 168, 83, 0.7)', + borderColor: '#34a853', + borderWidth: 1, + maxBarThickness: 24, + }; + savingsChart = new Chart(ctx, { - type: 'line', + type: cumulative ? 'line' : 'bar', data: { - datasets: [ - { - label: 'Period Savings', - data: periodSavingsData, - borderColor: '#34a853', - backgroundColor: 'rgba(52, 168, 83, 0.1)', - fill: true, - tension: 0.3, - pointRadius: dataPoints.length > 50 ? 0 : 3, - pointHoverRadius: 5, - yAxisID: 'y', - }, - { - label: 'Cumulative Savings', - data: cumulativeSavingsData, - borderColor: '#4285f4', - backgroundColor: 'rgba(66, 133, 244, 0.05)', - fill: false, - tension: 0.3, - borderDash: [5, 5], - pointRadius: dataPoints.length > 50 ? 0 : 2, - pointHoverRadius: 4, - yAxisID: 'y1', - }, - ], + datasets: [dataset], }, options: { responsive: true, @@ -443,6 +441,9 @@ function renderSavingsChart( display: true, min: axisMinMs, max: axisMaxMs, + // A bar chart pads the x-axis by default, which would + // push the period start off the axis edge (QA 2.2). + offset: false, grid: { display: false, }, @@ -464,62 +465,25 @@ function renderSavingsChart( maxTicksLimit: 6, callback: function(value: number | string) { const numValue = typeof value === 'string' ? parseFloat(value) : value; - return `$${numValue.toFixed(2)}`; - }, - }, - title: { - display: true, - text: `Savings per Period (${unitLabel(unit)})`, - }, - }, - y1: { - type: 'linear', - display: true, - position: 'right', - beginAtZero: true, - grid: { - drawOnChartArea: false, - }, - ticks: { - maxTicksLimit: 6, - callback: function(value: number | string) { - const numValue = typeof value === 'string' ? parseFloat(value) : value; - if (numValue >= 1000) { - return `$${(numValue / 1000).toFixed(1)}K`; - } - // Show 2 decimal places for non-integer float ticks to prevent - // distinct values collapsing to the same integer label when the - // auto-scaled domain is narrow (QA 2.4/2.5). - return Number.isInteger(numValue) - ? `$${numValue}` - : `$${numValue.toFixed(2)}`; + return formatCompactCurrency(numValue); }, }, title: { display: true, - text: 'Cumulative Savings', + text: cumulative ? 'Cumulative savings' : `Savings per period (${unitLabel(unit)})`, }, }, }, plugins: { legend: { - position: 'top', - labels: { - usePointStyle: true, - boxWidth: 8, - }, + display: false, }, tooltip: { callbacks: { label: function(context) { const raw = context.raw as ({ x: number; y: number } | number) | null | undefined; const value = (raw != null && typeof raw === 'object' ? raw.y : raw as number) || 0; - if (context.datasetIndex === 1) { - // Cumulative savings -- raw total, no rate suffix - return `${context.dataset.label}: $${value.toFixed(2)}`; - } - // Period Savings: 2 decimal places matching the KPI box (QA 2.3) - return `${context.dataset.label}: $${value.toFixed(2)}${suffix}`; + return `${context.dataset.label}: ${formatCurrency(value, '$', 2)}${suffix}`; }, }, }, @@ -573,6 +537,16 @@ export function initSavingsHistory(): void { refreshBtn.addEventListener('click', loadSavingsHistory); } + // Series toggle (issue #749): same dedup pattern as the unit dropdown. + const seriesSelect = document.getElementById('savings-series') as (HTMLSelectElement & { _seriesChangeHandler?: () => void }) | null; + if (seriesSelect) { + if (seriesSelect._seriesChangeHandler) { + seriesSelect.removeEventListener('change', seriesSelect._seriesChangeHandler); + } + seriesSelect._seriesChangeHandler = (): void => { void loadSavingsHistory(); }; + seriesSelect.addEventListener('change', seriesSelect._seriesChangeHandler); + } + // Wire unit dropdown. Using a named handler stored as a property so that // repeated calls to initSavingsHistory() don't stack duplicate listeners // (feedback_event_listener_dedup pattern). diff --git a/frontend/tests-e2e/savings-history-average.spec.ts b/frontend/tests-e2e/savings-history-average.spec.ts index 815de8075..b3e01225d 100644 --- a/frontend/tests-e2e/savings-history-average.spec.ts +++ b/frontend/tests-e2e/savings-history-average.spec.ts @@ -52,7 +52,7 @@ async function changePeriod(page: Page, period: string) { test.use({ timezoneId: 'UTC' }); for (const [period, count, average, totalFull] of [ - ['24h', 24, '$70.00/mo', '$240.00'], ['7d', 168, '$10.00/mo', '$1.68K'], + ['24h', 24, '$70.00/mo', '$240.00'], ['7d', 168, '$10.00/mo', '$1,680.00'], ['30d', 30, '$56.00/mo', '$300.00'], ['90d', 90, '$18.67/mo', '$900.00'], ] as const) { test(period + ' averages the requested window regardless of missing zero buckets', async ({ page }) => { @@ -66,7 +66,7 @@ for (const [period, count, average, totalFull] of [ await changePeriod(page, period); await expect(page.locator('#avg-savings-label')).toHaveText('Avg Monthly Savings'); await expect(page.locator('#avg-hourly-savings')).toHaveText(mode === 'full' ? '$10.00/mo' : average); - await expect(page.locator('#period-savings')).toHaveText(mode === 'full' ? totalFull : '$1.68K'); + await expect(page.locator('#period-savings')).toHaveText(mode === 'full' ? totalFull : '$1,680.00'); await expect(page.locator('#peak-savings')).toHaveText(mode === 'full' ? '$10.00/mo' : '$560.00/mo'); const request = fixture.requests[fixture.requests.length - 1]!; const ms = period === '24h' || period === '7d' ? 3600000 : 86400000; @@ -84,8 +84,8 @@ test('unit switches, refresh and empty/error states work on a narrow screen', as await changePeriod(page, '7d'); for (const [unit, label, average, total, peak] of [ ['hourly', 'Hourly', '$0.01/hr', '$2.30', '$0.77/hr'], - ['yearly', 'Yearly', '$120.00/yr', '$20.16K', '$6.72K/yr'], - ['monthly', 'Monthly', '$10.00/mo', '$1.68K', '$560.00/mo'], + ['yearly', 'Yearly', '$120.00/yr', '$20,160.00', '$6,720.00/yr'], + ['monthly', 'Monthly', '$10.00/mo', '$1,680.00', '$560.00/mo'], ]) { await page.locator('#savings-unit').selectOption(unit!); await expect(page.locator('#avg-savings-label')).toHaveText('Avg ' + label + ' Savings'); @@ -93,6 +93,12 @@ test('unit switches, refresh and empty/error states work on a narrow screen', as await expect(page.locator('#period-savings')).toHaveText(total!); await expect(page.locator('#peak-savings')).toHaveText(peak!); } + // Issue #749: the running total is its own view, re-queried on change. + const reload = page.waitForResponse(value => value.url().includes('/history/analytics?')); + await page.locator('#savings-series').selectOption('cumulative'); + await reload; + await expect(page.locator('#savings-history-chart')).toBeVisible(); + await page.locator('#savings-series').selectOption('period'); fixture.mode = 'zero'; await page.locator('#refresh-savings-btn').click(); await expect(page.locator('#avg-hourly-savings')).toHaveText('$0.00/mo'); From 33a18e5b47719913abf4bed6822546d84e687a40 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 21:00:15 +0000 Subject: [PATCH 3/3] fix(frontend): share formatCurrency's absent marker in formatCompactCurrency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main's #1376 (Refs #739) changed formatCurrency's absent marker to NO_VALUE ("—"); formatCompactCurrency still hardcoded "--". Delegate null/NaN/Infinity to formatCurrency so the two cannot drift again. Refs #749 Co-Authored-By: Claude Opus 5.5 --- frontend/src/utils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts index bf6fc0669..c844608d2 100644 --- a/frontend/src/utils.ts +++ b/frontend/src/utils.ts @@ -70,7 +70,8 @@ export function formatCurrency( */ export function formatCompactCurrency(value: number | null | undefined, digits = 2): string { if (value === null || value === undefined || !Number.isFinite(value)) { - return '--'; + // Delegate so both formatters always share the absent marker (NO_VALUE). + return formatCurrency(value); } return new Intl.NumberFormat('en-US', { style: 'currency',