diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index c08d3c664..69cc38ce9 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -1128,4 +1128,268 @@ describe('Dashboard Module', () => { document.body.removeChild(svg); }); }); + + // Issue #765: per-service savings-range bar chart. + describe('renderSavingsByService (issue #765)', () => { + // Import the public helpers from dashboard. The module is already + // loaded above via the jest.mock chain, so we can import directly. + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { renderSavingsByService, computeServiceStats } = require('../dashboard') as { + renderSavingsByService: (dataPoints: unknown[]) => void; + computeServiceStats: (dataPoints: unknown[]) => Map; + }; + + function buildDOM(): void { + const canvas = document.createElement('canvas'); + canvas.id = 'savings-by-service-chart'; + const empty = document.createElement('p'); + empty.id = 'savings-by-service-empty'; + empty.className = 'empty hidden'; + const section = document.createElement('section'); + section.id = 'savings-by-service-section'; + const h3 = document.createElement('h3'); + h3.textContent = 'Savings range by service'; + section.appendChild(h3); + section.appendChild(canvas); + section.appendChild(empty); + document.body.appendChild(section); + } + + beforeEach(() => { + document.body.innerHTML = ''; + jest.clearAllMocks(); + // Re-apply the recommendation mock resets from the outer beforeEach. + mockGroupRecsByCell.mockImplementation((recs: unknown[]) => new Map(recs.length ? [['cell-1', recs]] : [])); + mockPageLevelRange.mockImplementation((groups: Map) => { + if (groups.size === 0) return { savingsMin: 0, savingsMax: 0, cellCount: 0 }; + return { savingsMin: 300, savingsMax: 400, cellCount: groups.size }; + }); + mockFormatSavingsRange.mockImplementation((min: number, max: number) => min === max ? `$${min}` : `$${min} – $${max}`); + (api.getRecommendations as jest.Mock).mockResolvedValue([]); + }); + + // computeServiceStats unit tests. + describe('computeServiceStats', () => { + test('returns empty map for empty data points', () => { + const result = computeServiceStats([]); + expect(result.size).toBe(0); + }); + + test('returns empty map when all data points have no by_service', () => { + const result = computeServiceStats([ + { timestamp: 't1', total_savings: 100, total_upfront: 0, purchase_count: 1, cumulative_savings: 100 }, + ]); + expect(result.size).toBe(0); + }); + + test('accumulates min/max/sum/count correctly for a single service', () => { + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 50 } }, + { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 200 } }, + { timestamp: 't3', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, + ]; + const result = computeServiceStats(points); + expect(result.size).toBe(1); + const ec2 = result.get('ec2'); + expect(ec2?.min).toBe(50); + expect(ec2?.max).toBe(200); + expect(ec2?.sum).toBe(350); + expect(ec2?.count).toBe(3); + }); + + test('accumulates stats for multiple services independently', () => { + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100, rds: 50 } }, + { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 300, rds: 80 } }, + ]; + const result = computeServiceStats(points); + expect(result.size).toBe(2); + expect(result.get('ec2')?.min).toBe(100); + expect(result.get('ec2')?.max).toBe(300); + expect(result.get('rds')?.min).toBe(50); + expect(result.get('rds')?.max).toBe(80); + }); + + test('skips data points with missing by_service (omitempty)', () => { + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0 }, + { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 200 } }, + ]; + const result = computeServiceStats(points); + expect(result.get('ec2')?.count).toBe(1); + }); + + test('stores raw sample values for median computation', () => { + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 50 } }, + { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 200 } }, + { timestamp: 't3', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, + ]; + const result = computeServiceStats(points); + const ec2 = result.get('ec2'); + expect(ec2?.samples).toHaveLength(3); + expect(ec2?.samples).toContain(50); + expect(ec2?.samples).toContain(100); + expect(ec2?.samples).toContain(200); + }); + }); + + // renderSavingsByService DOM behaviour tests. + describe('DOM behaviour', () => { + test('shows empty state and hides canvas when no data points', () => { + buildDOM(); + renderSavingsByService([]); + const canvas = document.getElementById('savings-by-service-chart'); + const empty = document.getElementById('savings-by-service-empty'); + expect(canvas?.classList.contains('hidden')).toBe(true); + expect(empty?.classList.contains('hidden')).toBe(false); + }); + + test('shows empty state when all data points have zero savings', () => { + buildDOM(); + renderSavingsByService([ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 0 } }, + ]); + expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(true); + expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(false); + }); + + test('resets heading text to default when dataset becomes empty after a truncated render', () => { + buildDOM(); + // First render with 2 services to get the default heading. + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, + by_service: { ec2: 100, rds: 50 } }, + ]; + renderSavingsByService(points); + const h3 = document.querySelector('#savings-by-service-section h3') as HTMLElement; + h3.textContent = 'Savings range by service (+3 more)'; // simulate stale suffix + // Second render with empty data -- heading must be reset. + renderSavingsByService([]); + expect(h3.textContent).toBe('Savings range by service'); + }); + + test('renders chart with exactly two services when two services have positive savings', () => { + buildDOM(); + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100, rds: 50 } }, + { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 300, rds: 80 } }, + ]; + renderSavingsByService(points); + // Chart must be constructed (canvas visible, empty hidden). + expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(false); + expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(true); + // Chart.js was called with both services as labels. + const chartCtor = Chart as unknown as jest.Mock; + const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1]; + const chartData = lastCall?.[1] as { data: { labels: string[] } }; + expect(chartData.data.labels).toHaveLength(2); + expect(chartData.data.labels).toContain('ec2'); + expect(chartData.data.labels).toContain('rds'); + }); + + test('bar floor dataset uses min, range dataset uses (max - min)', () => { + buildDOM(); + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, + { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 400 } }, + ]; + renderSavingsByService(points); + const chartCtor = Chart as unknown as jest.Mock; + const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1]; + const datasets = (lastCall?.[1] as { data: { datasets: { label: string; data: number[] }[] } }).data.datasets; + const floorDs = datasets.find((d) => d.label === 'Floor'); + const rangeDs = datasets.find((d) => d.label === 'Range'); + expect(floorDs?.data[0]).toBe(100); // min + expect(rangeDs?.data[0]).toBe(300); // max - min + }); + + test('services are sorted by max savings descending', () => { + buildDOM(); + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, + by_service: { ec2: 100, rds: 500, lambda: 50 } }, + ]; + renderSavingsByService(points); + const chartCtor = Chart as unknown as jest.Mock; + const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1]; + const labels = (lastCall?.[1] as { data: { labels: string[] } }).data.labels; + // rds has max 500, ec2 100, lambda 50 — rds must be first. + expect(labels[0]).toBe('rds'); + expect(labels[1]).toBe('ec2'); + expect(labels[2]).toBe('lambda'); + }); + + test('destroys existing chart instance before re-rendering', () => { + buildDOM(); + const mockDestroyA = jest.fn(); + (Chart as unknown as jest.Mock).mockImplementationOnce(() => ({ destroy: mockDestroyA })); + + const points = [ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, + ]; + renderSavingsByService(points); + // Second call must destroy the first chart. + renderSavingsByService(points); + expect(mockDestroyA).toHaveBeenCalled(); + }); + + test('no-ops gracefully when canvas is missing from DOM', () => { + // No buildDOM() call — canvas absent. + expect(() => renderSavingsByService([ + { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, + ])).not.toThrow(); + }); + }); + + // Filter chip change re-renders via loadSavingsTrendChart. + describe('filter integration via loadSavingsTrendChart', () => { + beforeEach(() => { + buildDOM(); + const canvas = document.createElement('canvas'); + canvas.id = 'savings-trend-chart'; + const empty = document.createElement('div'); + empty.id = 'savings-trend-empty'; + empty.className = 'hidden'; + document.body.appendChild(canvas); + document.body.appendChild(empty); + }); + + test('re-renders bar chart when trend chart is re-fetched due to filter change', async () => { + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ + data_points: [ + { timestamp: 't1', total_savings: 100, total_upfront: 0, purchase_count: 1, cumulative_savings: 100, + by_service: { ec2: 100, rds: 50 } }, + ], + }); + + await loadSavingsTrendChart(); + + const barCanvas = document.getElementById('savings-by-service-chart'); + expect(barCanvas?.classList.contains('hidden')).toBe(false); + const chartCtor = Chart as unknown as jest.Mock; + // At least one Chart call must have been for the bar chart. + const barChartCall = chartCtor.mock.calls.find( + (call: unknown[]) => (call[1] as { type: string })?.type === 'bar' + ); + expect(barChartCall).toBeDefined(); + }); + + test('re-renders bar chart empty state when analytics data is empty', async () => { + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] }); + + await loadSavingsTrendChart(); + + expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(true); + expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(false); + }); + + // Note: provider is intentionally NOT forwarded to getSavingsAnalytics + // because the /history/analytics handler does not yet support a + // provider-scoped query (see dashboard.ts comment near the fetch call + // for the original CR finding and the deliberate decision). A test + // asserting the negative would be brittle; the comment in the + // production code is the source of truth. + }); + }); }); diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 55d9f6364..dac651b8d 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -21,6 +21,27 @@ Chart.register(...registerables); let savingsTrendChart: Chart | null = null; let savingsTrendRange: '7' | '30' | '90' | 'all' = '90'; +// Chart instance for the per-service savings-range bar chart (issue #765). +let savingsByServiceChart: Chart | null = null; + +// Maximum number of services to show in the bar chart before truncating. +const SAVINGS_BY_SERVICE_MAX = 10; + +// Default palette for per-service bars. Cycles if there are more services +// than colours; alpha variant is computed inline so the array stays short. +const SERVICE_BAR_COLORS = [ + '#1a73e8', // blue + '#34a853', // green + '#fbbc04', // yellow + '#ea4335', // red + '#9c27b0', // purple + '#00bcd4', // cyan + '#ff5722', // deep-orange + '#607d8b', // blue-grey + '#795548', // brown + '#4caf50', // light-green +]; + // In-memory index of the currently-rendered upcoming purchases, keyed by // execution_id. The "View Details" affordance renders from this — the // /api/dashboard/upcoming response already carries every field the @@ -299,7 +320,7 @@ function attachSparkline(key: string, values: readonly number[]): void { svg.appendChild(polyline); } -export const __test__ = { sparklinePoints, attachSparkline }; +export const __test__ = { sparklinePoints, attachSparkline, computeServiceStats }; function renderSavingsChart(byService: Record): void { const ctx = document.getElementById('savings-chart') as HTMLCanvasElement | null; @@ -612,6 +633,190 @@ async function cancelScheduledPurchase(executionId: string): Promise { } } +/** + * Per-service savings statistics derived from a window of data points. + * Exported for unit testing only. + */ +export interface ServiceSavingsStats { + min: number; + max: number; + sum: number; + count: number; + samples: number[]; +} + +/** + * Compute per-service savings stats from an array of data points. Each + * data point carries a by_service map of { serviceName: savingsValue }. + * Points with no by_service entry (omitempty from backend) are skipped. + * Exported for unit testing. + */ +export function computeServiceStats( + dataPoints: readonly SavingsDataPoint[], +): Map { + const stats = new Map(); + for (const dp of dataPoints) { + if (!dp.by_service) continue; + for (const [svc, val] of Object.entries(dp.by_service)) { + if (typeof val !== 'number') continue; + const existing = stats.get(svc); + if (existing) { + existing.min = Math.min(existing.min, val); + existing.max = Math.max(existing.max, val); + existing.sum += val; + existing.count += 1; + existing.samples.push(val); + } else { + stats.set(svc, { min: val, max: val, sum: val, count: 1, samples: [val] }); + } + } + } + return stats; +} + +/** + * Compute the median of a sorted sample array. + * Returns 0 for an empty array. + */ +function medianOf(values: number[]): number { + if (values.length === 0) return 0; + const sorted = [...values].sort((a, b) => a - b); + const mid = Math.floor(sorted.length / 2); + return sorted.length % 2 === 0 ? (sorted[mid - 1]! + sorted[mid]!) / 2 : sorted[mid]!; +} + +/** + * Render the per-service savings-range stacked bar chart (issue #765). + * Accepts the same data_points array as the trend line chart. + * + * Each bar is split into two stacked datasets: + * - "Floor" (solid): height = min savings observed across the window. + * - "Range" (translucent 35% opacity): height = max - min (the upside). + * + * Services are sorted by max savings descending; the top SAVINGS_BY_SERVICE_MAX + * are shown. When truncated, the section heading notes "+N more". + * + * Empty state: when no service has positive savings, the canvas is hidden and + * the empty-state paragraph is shown. + */ +export function renderSavingsByService(dataPoints: readonly SavingsDataPoint[]): void { + const canvas = document.getElementById('savings-by-service-chart') as HTMLCanvasElement | null; + const emptyEl = document.getElementById('savings-by-service-empty'); + const section = document.getElementById('savings-by-service-section'); + if (!canvas) return; + + // Destroy existing chart before rebuilding. + if (savingsByServiceChart) { + savingsByServiceChart.destroy(); + savingsByServiceChart = null; + } + + const stats = computeServiceStats(dataPoints); + const heading = section?.querySelector('h3'); + + // Filter to services with positive savings, then sort by max desc. + const positive = Array.from(stats.entries()).filter(([, s]) => s.max > 0); + positive.sort((a, b) => b[1].max - a[1].max); + + if (positive.length === 0) { + if (heading) heading.textContent = 'Savings range by service'; + canvas.classList.add('hidden'); + emptyEl?.classList.remove('hidden'); + return; + } + + canvas.classList.remove('hidden'); + emptyEl?.classList.add('hidden'); + + // Cap at maximum and update heading with "+N more" if truncated. + const truncated = positive.length > SAVINGS_BY_SERVICE_MAX; + const visible = positive.slice(0, SAVINGS_BY_SERVICE_MAX); + if (heading) { + heading.textContent = truncated + ? `Savings range by service (+${positive.length - SAVINGS_BY_SERVICE_MAX} more)` + : 'Savings range by service'; + } + + const labels = visible.map(([svc]) => svc); + const floorData = visible.map(([, s]) => s.min); + const rangeData = visible.map(([, s]) => s.max - s.min); + const totalSavings = Array.from(stats.values()).reduce((acc, s) => acc + s.max, 0); + + // Assign a colour per service (cycles if more than palette length). + const bgColors = visible.map((_, i) => SERVICE_BAR_COLORS[i % SERVICE_BAR_COLORS.length] ?? '#1a73e8'); + const rangeColors = bgColors.map(c => { + // Convert solid hex to rgba at 0.35 opacity for the range segment. + const hex = c.replace('#', ''); + const r = parseInt(hex.substring(0, 2), 16); + const g = parseInt(hex.substring(2, 4), 16); + const b = parseInt(hex.substring(4, 6), 16); + return `rgba(${r},${g},${b},0.35)`; + }); + + savingsByServiceChart = new Chart(canvas, { + type: 'bar', + data: { + labels, + datasets: [ + { + label: 'Floor', + data: floorData, + backgroundColor: bgColors, + borderRadius: { topLeft: 0, topRight: 0, bottomLeft: 4, bottomRight: 4 }, + stack: 'savings', + }, + { + label: 'Range', + data: rangeData, + backgroundColor: rangeColors, + borderRadius: { topLeft: 4, topRight: 4, bottomLeft: 0, bottomRight: 0 }, + stack: 'savings', + }, + ], + }, + options: { + responsive: true, + maintainAspectRatio: false, + plugins: { + legend: { + display: true, + position: 'top', + labels: { boxWidth: 12 }, + }, + tooltip: { + callbacks: { + label: (ctx) => { + const svc = ctx.label ?? ''; + const s = stats.get(svc); + if (!s) return ''; + const pct = totalSavings > 0 ? ((s.max / totalSavings) * 100).toFixed(1) : '0.0'; + const med = medianOf(s.samples); + return [ + `Service: ${svc}`, + `Min: $${s.min.toLocaleString()}`, + `Median: $${med.toLocaleString()}`, + `Max: $${s.max.toLocaleString()}`, + `Samples: ${s.count}`, + `% of total: ${pct}%`, + ]; + }, + }, + }, + }, + scales: { + x: { + stacked: true, + }, + y: { + stacked: true, + beginAtZero: true, + ticks: { callback: (v) => '$' + (v as number).toLocaleString() }, + }, + }, + }, + }); +} + /** * Format a millisecond timestamp for the savings-trend x-axis tick label. * Exported for unit testing. @@ -726,11 +931,18 @@ export async function loadSavingsTrendChart(): Promise { } if (savingsTrendChart) { savingsTrendChart.destroy(); savingsTrendChart = null; } attachSparkline('ytd', []); + // Show empty state for the per-service bar chart as well. + renderSavingsByService([]); return; } canvas.classList.remove('hidden'); empty?.classList.add('hidden'); + // Per-service savings-range bar chart (issue #765). Shares the same + // data_points response so no extra fetch is needed. The YTD sparkline + // is already populated earlier in this function via attachSparkline. + renderSavingsByService(data.data_points); + if (savingsTrendChart) savingsTrendChart.destroy(); savingsTrendChart = new Chart(canvas, { @@ -786,6 +998,8 @@ export async function loadSavingsTrendChart(): Promise { empty.textContent = 'Savings history is not available yet.'; empty.classList.remove('hidden'); } + // Degrade the per-service bar chart to its empty state too. + renderSavingsByService([]); } } diff --git a/frontend/src/index.html b/frontend/src/index.html index 57531e07f..5b8c3a2d2 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -93,6 +93,11 @@

Savings over time

+
+

Savings range by service

+ + +

Potential Savings by Service