diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts
index 58d186c74..c24e2c735 100644
--- a/frontend/src/__tests__/dashboard.test.ts
+++ b/frontend/src/__tests__/dashboard.test.ts
@@ -105,7 +105,10 @@ describe('Dashboard Module', () => {
// Reset DOM
document.body.innerHTML = `
-
+ Potential savings range per service
+
+
+
`;
@@ -174,7 +177,7 @@ describe('Dashboard Module', () => {
expect(summary?.innerHTML).toContain('YTD Savings');
});
- test('renders savings chart', async () => {
+ test('renders the merged per-service savings chart', async () => {
(api.getDashboardSummary as jest.Mock).mockResolvedValue({
potential_monthly_savings: 1000,
by_service: {
@@ -185,28 +188,37 @@ describe('Dashboard Module', () => {
(api.getUpcomingPurchases as jest.Mock).mockResolvedValue({
purchases: []
});
+ // Range chart is driven by recs; supply two ec2 options + one rds.
+ (api.getRecommendations as jest.Mock).mockResolvedValue([
+ { service: 'ec2', savings: 400, term: 1, payment: 'no-upfront' },
+ { service: 'ec2', savings: 600, term: 3, payment: 'all-upfront' },
+ { service: 'rds', savings: 300, term: 1, payment: 'no-upfront' },
+ ]);
await loadDashboard();
expect(Chart).toHaveBeenCalled();
- expect(state.setSavingsChart).toHaveBeenCalled();
});
- test('destroys existing chart before creating new one', async () => {
- const mockChart = { destroy: jest.fn() };
- (state.getSavingsChart as jest.Mock).mockReturnValue(mockChart);
-
+ test('destroys existing merged chart before creating new one', async () => {
(api.getDashboardSummary as jest.Mock).mockResolvedValue({
potential_monthly_savings: 1000,
- by_service: {}
+ by_service: { ec2: { potential_savings: 500, current_savings: 200 } }
});
(api.getUpcomingPurchases as jest.Mock).mockResolvedValue({
purchases: []
});
+ (api.getRecommendations as jest.Mock).mockResolvedValue([
+ { service: 'ec2', savings: 500, term: 1, payment: 'no-upfront' },
+ ]);
+ // First render builds a chart; second render must destroy it.
+ await loadDashboard();
+ const results = (Chart as unknown as jest.Mock).mock.results;
+ const firstChart = results[results.length - 1]?.value as { destroy: jest.Mock };
await loadDashboard();
- expect(mockChart.destroy).toHaveBeenCalled();
+ expect(firstChart.destroy).toHaveBeenCalled();
});
test('renders upcoming purchases', async () => {
@@ -700,7 +712,7 @@ describe('Dashboard Module', () => {
});
// #304: summaryData.by_service missing entirely (null/undefined from
- // backend). renderSavingsChart receives `undefined || {}` = {} which
+ // backend). renderSavingsByService receives `undefined || {}` = {} which
// is safe; verify no throw and the error banner does not appear.
test('#304: summaryData missing by_service field does not throw', async () => {
(api.getDashboardSummary as jest.Mock).mockResolvedValue({
@@ -1134,10 +1146,12 @@ describe('Dashboard Module', () => {
// 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, computeServiceStatsFromRecs } = require('../dashboard') as {
- renderSavingsByService: (recs: unknown[], filterDesc?: string) => void;
+ const { renderSavingsByService, computeServiceStats, computeServiceStatsFromRecs, darkenHexColor, parseHexColor } = require('../dashboard') as {
+ renderSavingsByService: (recs: unknown[], byService?: Record, filterDesc?: string) => void;
computeServiceStats: (dataPoints: unknown[]) => Map;
computeServiceStatsFromRecs: (recs: unknown[]) => Map;
+ darkenHexColor: (hex: string, factor?: number) => string;
+ parseHexColor: (hex: string) => { r: number; g: number; b: number };
};
function buildDOM(): void {
@@ -1393,6 +1407,57 @@ describe('Dashboard Module', () => {
expect(rangeDs?.data[0]).toBe(300); // max - min
});
+ // Issue #908: merged chart draws a current-savings underlay per service.
+ test('renders a current (committed) dataset from byService.current_savings', () => {
+ buildDOM();
+ renderSavingsByService(
+ [rec('ec2', 100, 1, 'no_upfront'), rec('ec2', 400, 3, 'all_upfront')],
+ { ec2: { potential_savings: 400, current_savings: 250 } },
+ );
+ 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[]; backgroundColor: string[]; stack: string }[] };
+ }).data.datasets;
+ const currentDs = datasets.find((d) => d.label === 'Current (committed)');
+ expect(currentDs).toBeDefined();
+ expect(currentDs?.data[0]).toBe(250);
+ // Current bar lives in its own stack so it sits beside the potential range.
+ expect(currentDs?.stack).toBe('current');
+ });
+
+ test('current underlay uses a DARKER variant of each service potential hue', () => {
+ buildDOM();
+ renderSavingsByService(
+ [rec('ec2', 500)],
+ { ec2: { potential_savings: 500, current_savings: 200 } },
+ );
+ 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; backgroundColor: string[] }[] };
+ }).data.datasets;
+ const floorColor = datasets.find((d) => d.label === 'Min potential')?.backgroundColor[0] as string;
+ const currentColor = datasets.find((d) => d.label === 'Current (committed)')?.backgroundColor[0] as string;
+ // The current colour is the darkened form of the floor (potential) colour.
+ expect(currentColor).toBe(darkenHexColor(floorColor));
+ // And it is genuinely darker: each channel sum is lower.
+ const sum = (hex: string): number => { const c = parseHexColor(hex); return c.r + c.g + c.b; };
+ expect(sum(currentColor)).toBeLessThan(sum(floorColor));
+ });
+
+ test('current bar defaults to 0 for a service absent from byService', () => {
+ buildDOM();
+ renderSavingsByService([rec('rds', 300)], {}); // no rds entry
+ 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 currentDs = datasets.find((d) => d.label === 'Current (committed)');
+ expect(currentDs?.data[0]).toBe(0);
+ });
+
test('services are sorted by max potential savings descending', () => {
buildDOM();
// ec2: max=200, rds: max=500, lambda: max=50 -- expected order: rds, ec2, lambda.
@@ -1429,7 +1494,7 @@ describe('Dashboard Module', () => {
// Issue #867: filter-aware empty state.
test('empty state shows generic text when no filter is active', () => {
buildDOM();
- renderSavingsByService([], '');
+ renderSavingsByService([], {}, '');
const empty = document.getElementById('savings-by-service-empty');
expect(empty?.classList.contains('hidden')).toBe(false);
expect(empty?.textContent).toBe('No positive potential savings found for current recommendations.');
@@ -1437,7 +1502,7 @@ describe('Dashboard Module', () => {
test('empty state mentions filter when provider chip is active and result is empty', () => {
buildDOM();
- renderSavingsByService([], 'AWS');
+ renderSavingsByService([], {}, 'AWS');
const empty = document.getElementById('savings-by-service-empty');
expect(empty?.classList.contains('hidden')).toBe(false);
expect(empty?.textContent).toContain('AWS');
@@ -1446,7 +1511,7 @@ describe('Dashboard Module', () => {
test('empty state mentions filter when account chip is active and result is empty', () => {
buildDOM();
- renderSavingsByService([], 'uuid-acct-1');
+ renderSavingsByService([], {}, 'uuid-acct-1');
const empty = document.getElementById('savings-by-service-empty');
expect(empty?.classList.contains('hidden')).toBe(false);
expect(empty?.textContent).toContain('uuid-acct-1');
@@ -1457,7 +1522,7 @@ describe('Dashboard Module', () => {
// First render with data -- chart shown, empty hidden.
renderSavingsByService([rec('ec2', 100)]);
// Second render with filter-narrowed empty result.
- renderSavingsByService([], 'AWS, uuid-acct-2');
+ renderSavingsByService([], {}, 'AWS, uuid-acct-2');
const empty = document.getElementById('savings-by-service-empty');
expect(empty?.classList.contains('hidden')).toBe(false);
expect(empty?.textContent).toContain('AWS, uuid-acct-2');
@@ -1471,13 +1536,7 @@ describe('Dashboard Module', () => {
summaryEl.id = 'summary';
const upcomingEl = document.createElement('div');
upcomingEl.id = 'upcoming-list';
- const savingsChartSection = document.createElement('section');
- savingsChartSection.id = 'savings-chart-section';
- const savingsCanvas = document.createElement('canvas');
- savingsCanvas.id = 'savings-chart';
- savingsChartSection.appendChild(savingsCanvas);
document.body.appendChild(summaryEl);
- document.body.appendChild(savingsChartSection);
document.body.appendChild(upcomingEl);
(api.getDashboardSummary as jest.Mock).mockResolvedValue({
@@ -1497,6 +1556,71 @@ describe('Dashboard Module', () => {
expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(false);
expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(true);
});
+
+ // Issue #908: the merged chart must keep honoring the topbar chips.
+ // loadDashboard re-runs on every chip change (via the state subscribers
+ // wired in setupDashboardHandlers), so a second load with new filter
+ // results must re-render the chart with the new data.
+ test('re-renders the merged chart when the filter changes between loads', async () => {
+ buildDOM();
+ const summaryEl = document.createElement('section');
+ summaryEl.id = 'summary';
+ const upcomingEl = document.createElement('div');
+ upcomingEl.id = 'upcoming-list';
+ document.body.appendChild(summaryEl);
+ document.body.appendChild(upcomingEl);
+
+ (api.getDashboardSummary as jest.Mock).mockResolvedValue({
+ potential_monthly_savings: 0, total_recommendations: 1,
+ active_commitments: 0, committed_monthly: 0, target_coverage: 80,
+ ytd_savings: 0, by_service: { ec2: { potential_savings: 150, current_savings: 90 } },
+ });
+ (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+ (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] });
+
+ // First load: AWS/ec2 result.
+ (api.getRecommendations as jest.Mock).mockResolvedValue([rec('ec2', 150)]);
+ await loadDashboard();
+ const chartCtor = Chart as unknown as jest.Mock;
+ const lastChartData = (): { labels: string[] } =>
+ (chartCtor.mock.calls[chartCtor.mock.calls.length - 1]?.[1] as { data: { labels: string[] } }).data;
+ expect(lastChartData().labels).toEqual(['ec2']);
+
+ // Filter changes -> new recs -> second load must re-render with rds.
+ (api.getRecommendations as jest.Mock).mockResolvedValue([rec('rds', 220)]);
+ await loadDashboard();
+ expect(lastChartData().labels).toEqual(['rds']);
+ });
+ });
+
+ // Issue #908: colour-derivation helpers for the current-savings underlay.
+ describe('colour helpers (issue #908)', () => {
+ test('parseHexColor parses #rrggbb', () => {
+ expect(parseHexColor('#1a73e8')).toEqual({ r: 26, g: 115, b: 232 });
+ });
+
+ test('parseHexColor tolerates a missing leading hash', () => {
+ expect(parseHexColor('34a853')).toEqual({ r: 52, g: 168, b: 83 });
+ });
+
+ test('parseHexColor falls back to the default blue for malformed input', () => {
+ expect(parseHexColor('not-a-color')).toEqual({ r: 26, g: 115, b: 232 });
+ });
+
+ test('darkenHexColor returns a strictly darker same-hue colour', () => {
+ const base = '#34a853';
+ const darker = darkenHexColor(base);
+ expect(darker).toMatch(/^#[0-9a-f]{6}$/);
+ const sum = (hex: string): number => { const c = parseHexColor(hex); return c.r + c.g + c.b; };
+ expect(sum(darker)).toBeLessThan(sum(base));
+ // 30% reduction by default (factor 0.7): green channel 168 -> ~118.
+ expect(parseHexColor(darker).g).toBe(Math.round(168 * 0.7));
+ });
+
+ test('darkenHexColor honours an explicit factor', () => {
+ // factor 0.5 halves each channel.
+ expect(darkenHexColor('#646464', 0.5)).toBe('#323232');
+ });
});
});
});
diff --git a/frontend/src/__tests__/html.test.ts b/frontend/src/__tests__/html.test.ts
index 668253945..cfc350a72 100644
--- a/frontend/src/__tests__/html.test.ts
+++ b/frontend/src/__tests__/html.test.ts
@@ -135,17 +135,25 @@ describe('HTML Structure', () => {
expect(summary).toBeTruthy();
});
- test('has savings chart section', () => {
- const chartSection = document.getElementById('savings-chart-section');
+ // Issue #908 merged the two per-service charts into one; the standalone
+ // #savings-chart-section / #savings-chart "Potential Savings by Service"
+ // chart was removed in favour of the merged range chart below.
+ test('has the merged per-service savings range section', () => {
+ const chartSection = document.getElementById('savings-by-service-section');
expect(chartSection).toBeTruthy();
});
- test('has savings chart canvas', () => {
- const canvas = document.getElementById('savings-chart');
+ test('has the merged per-service savings range canvas', () => {
+ const canvas = document.getElementById('savings-by-service-chart');
expect(canvas).toBeTruthy();
expect(canvas?.tagName.toLowerCase()).toBe('canvas');
});
+ test('no longer renders the old standalone savings chart (issue #908)', () => {
+ expect(document.getElementById('savings-chart-section')).toBeNull();
+ expect(document.getElementById('savings-chart')).toBeNull();
+ });
+
test('has upcoming purchases section', () => {
const upcoming = document.getElementById('upcoming-purchases');
expect(upcoming).toBeTruthy();
diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts
index abb0689af..01c4ae89c 100644
--- a/frontend/src/dashboard.ts
+++ b/frontend/src/dashboard.ts
@@ -16,8 +16,8 @@ import { showSkeletonTiles, showSkeletonBlock, teardownSkeleton } from './lib/sk
// Register Chart.js components
Chart.register(...registerables);
-// Separate Chart instance for the trend widget so renderSavingsChart's
-// state.savingsChart doesn't conflict.
+// Separate Chart instance for the trend widget so it doesn't conflict with
+// the per-service savings chart instance below.
let savingsTrendChart: Chart | null = null;
let savingsTrendRange: '7' | '30' | '90' | 'all' = '90';
@@ -42,6 +42,38 @@ const SERVICE_BAR_COLORS = [
'#4caf50', // light-green
];
+/**
+ * Parse a #rrggbb hex string into its r/g/b components. Falls back to the
+ * default service bar blue (#1a73e8) for malformed input so the chart never
+ * renders a NaN colour. Exported for unit testing.
+ */
+export function parseHexColor(hex: string): { r: number; g: number; b: number } {
+ const m = /^#?([0-9a-fA-F]{6})$/.exec(hex.trim());
+ const clean = m ? m[1]! : '1a73e8';
+ return {
+ r: parseInt(clean.substring(0, 2), 16),
+ g: parseInt(clean.substring(2, 4), 16),
+ b: parseInt(clean.substring(4, 6), 16),
+ };
+}
+
+/**
+ * Derive a darker shade of a base #rrggbb colour by reducing its perceived
+ * lightness. Used for the current-savings underlay so it reads as the
+ * "already-realized" portion beneath the lighter potential-range bar of the
+ * same hue (issue #908). The factor (default 0.7 → ~30% darker) is applied
+ * multiplicatively per channel so the hue is preserved; the result is
+ * programmatic, not hardcoded per service. Exported for unit testing.
+ */
+export function darkenHexColor(hex: string, factor = 0.7): string {
+ const { r, g, b } = parseHexColor(hex);
+ const scale = (c: number): string => {
+ const v = Math.max(0, Math.min(255, Math.round(c * factor)));
+ return v.toString(16).padStart(2, '0');
+ };
+ return `#${scale(r)}${scale(g)}${scale(b)}`;
+}
+
// 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
@@ -168,12 +200,13 @@ export async function loadDashboard(): Promise {
}
// Build a human-readable filter description for filter-aware empty states
- // on both Home charts. Mirrors the pattern from loadSavingsTrendChart (#747).
+ // on the Home chart. Mirrors the pattern from loadSavingsTrendChart (#747).
const filterDesc = buildFilterDesc(currentProvider, currentAccountIDs);
renderDashboardSummary(summaryData!, recs);
- renderSavingsChart(summaryData!.by_service || {}, filterDesc);
- renderSavingsByService(recs, filterDesc);
+ // Single merged per-service chart (#908): potential range from recs +
+ // current-savings underlay from the summary's by_service map.
+ renderSavingsByService(recs, summaryData!.by_service || {}, filterDesc);
renderUpcomingPurchases(upcomingData?.purchases || []);
// Load the savings-over-time widget independently -- failure shouldn't
// block the rest of the dashboard (e.g. analytics not configured).
@@ -340,86 +373,6 @@ function attachSparkline(key: string, values: readonly number[]): void {
export const __test__ = { sparklinePoints, attachSparkline, computeServiceStats };
-function renderSavingsChart(byService: Record, filterDesc = ''): void {
- const ctx = document.getElementById('savings-chart') as HTMLCanvasElement | null;
- if (!ctx) return;
-
- const labels = Object.keys(byService);
- const potentialSavings = labels.map(s => byService[s]?.potential_savings || 0);
- const currentSavings = labels.map(s => byService[s]?.current_savings || 0);
-
- const existingChart = state.getSavingsChart();
- if (existingChart) {
- existingChart.destroy();
- state.setSavingsChart(null);
- }
-
- // No data → hide the canvas and render an empty-state message so the
- // chart doesn't render with a synthetic $0–$1 y-axis.
- // When a filter is active, mention it so the user understands why the
- // chart is blank (mirrors the savings-trend empty-state pattern from #747).
- const section = ctx.parentElement;
- let emptyState = section?.querySelector('.chart-empty');
- if (labels.length === 0) {
- ctx.classList.add('hidden');
- const emptyText = filterDesc
- ? `No savings data for the selected filter (${filterDesc}).`
- : 'No savings data yet. Add accounts and wait for recommendations.';
- if (section && !emptyState) {
- emptyState = document.createElement('p');
- emptyState.className = 'chart-empty empty';
- section.appendChild(emptyState);
- }
- if (emptyState) emptyState.textContent = emptyText;
- return;
- }
- // Data is back — restore the canvas and remove any stale empty state.
- ctx.classList.remove('hidden');
- emptyState?.remove();
-
- const chart = new Chart(ctx, {
- type: 'bar',
- data: {
- labels: labels,
- datasets: [
- {
- label: 'Potential Savings',
- data: potentialSavings,
- backgroundColor: '#fbbc04',
- borderRadius: 4
- },
- {
- label: 'Current Savings',
- data: currentSavings,
- backgroundColor: '#34a853',
- borderRadius: 4
- }
- ]
- },
- options: {
- responsive: true,
- maintainAspectRatio: false,
- scales: {
- y: {
- beginAtZero: true,
- ticks: {
- callback: (value) => '$' + value.toLocaleString()
- }
- }
- },
- plugins: {
- tooltip: {
- callbacks: {
- label: (context) => `${context.dataset.label}: $${(context.raw as number).toLocaleString()}/mo`
- }
- }
- }
- }
- });
-
- state.setSavingsChart(chart);
-}
-
function renderUpcomingPurchases(purchases: UpcomingPurchase[]): void {
const container = document.getElementById('upcoming-list');
if (!container) return;
@@ -755,20 +708,34 @@ export function computeServiceStatsFromRecs(
/**
- * Render the per-service potential-savings range stacked bar chart (issue #769).
- * Accepts the recommendations array from loadDashboard.
+ * Render the single merged per-service savings chart (issues #769 + #908).
+ *
+ * Each service category shows two grouped bars:
+ * - Potential range (stacked): "Min potential" (solid hue) + "Upside"
+ * (same hue at 35% opacity, = max - min), derived from the
+ * recommendations list via computeServiceStatsFromRecs.
+ * - Current (committed) savings (single bar) in a DARKER shade of the
+ * same per-service hue, sourced from the dashboard summary's
+ * by_service[svc].current_savings (now populated by the backend, #908).
+ * This reads as the already-realized savings sitting beneath the
+ * potential range of the same colour family.
*
- * Each bar is split into two stacked datasets:
- * - "Min potential" (solid): height = min potential savings across recommendation rows for that service.
- * - "Upside" (translucent 35% opacity): height = max - min (the additional potential).
+ * This replaces the old standalone grouped "Potential Savings by Service"
+ * chart (renderSavingsChart) which has been removed; the current-vs-potential
+ * comparison now lives entirely in this one range chart.
*
- * Services are sorted by max potential savings descending; the top SAVINGS_BY_SERVICE_MAX
- * are shown. When truncated, the section heading notes "+N more".
+ * Services are sorted by max potential savings descending; the top
+ * SAVINGS_BY_SERVICE_MAX are shown. When truncated, the section heading notes
+ * "+N more".
*
- * Empty state: when no recommendations are available, the canvas is hidden and
- * the empty-state paragraph is shown.
+ * Empty state: when no recommendations have positive savings, the canvas is
+ * hidden and the empty-state paragraph is shown.
*/
-export function renderSavingsByService(recs: readonly LocalRecommendation[], filterDesc = ''): void {
+export function renderSavingsByService(
+ recs: readonly LocalRecommendation[],
+ byService: Record = {},
+ filterDesc = '',
+): 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');
@@ -816,18 +783,23 @@ export function renderSavingsByService(recs: readonly LocalRecommendation[], fil
const labels = visible.map(([svc]) => svc);
const floorData = visible.map(([, s]) => s.min);
const rangeData = visible.map(([, s]) => s.max - s.min);
+ // Current (committed) savings per service from the summary's by_service map.
+ // Defaults to 0 for services the summary doesn't carry (the bar collapses,
+ // which is the correct "nothing committed yet" visual).
+ const currentData = visible.map(([svc]) => byService[svc]?.current_savings ?? 0);
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);
+ const { r, g, b } = parseHexColor(c);
return `rgba(${r},${g},${b},0.35)`;
});
+ // Current-savings bar uses a darker variant of the same hue, derived
+ // programmatically (not hardcoded per service) so the "already-realized"
+ // bar always matches its service's colour family (#908).
+ const currentColors = bgColors.map(c => darkenHexColor(c));
savingsByServiceChart = new Chart(canvas, {
type: 'bar',
@@ -839,14 +811,21 @@ export function renderSavingsByService(recs: readonly LocalRecommendation[], fil
data: floorData,
backgroundColor: bgColors,
borderRadius: { topLeft: 0, topRight: 0, bottomLeft: 4, bottomRight: 4 },
- stack: 'savings',
+ stack: 'potential',
},
{
label: 'Upside',
data: rangeData,
backgroundColor: rangeColors,
borderRadius: { topLeft: 4, topRight: 4, bottomLeft: 0, bottomRight: 0 },
- stack: 'savings',
+ stack: 'potential',
+ },
+ {
+ label: 'Current (committed)',
+ data: currentData,
+ backgroundColor: currentColors,
+ borderRadius: 4,
+ stack: 'current',
},
],
},
@@ -865,6 +844,12 @@ export function renderSavingsByService(recs: readonly LocalRecommendation[], fil
const svc = ctx.label ?? '';
const s = stats.get(svc);
if (!s) return '';
+ // The current-savings dataset gets a focused, single-line
+ // tooltip; the range datasets share the full breakdown.
+ if (ctx.dataset.stack === 'current') {
+ const current = byService[svc]?.current_savings ?? 0;
+ return `Current (committed): $${current.toLocaleString()}`;
+ }
const pct = totalSavings > 0 ? ((s.max / totalSavings) * 100).toFixed(1) : '0.0';
const lines = [
`Service: ${svc}`,
@@ -883,10 +868,12 @@ export function renderSavingsByService(recs: readonly LocalRecommendation[], fil
scales: {
x: {
stacked: true,
+ title: { display: true, text: 'Service' },
},
y: {
stacked: true,
beginAtZero: true,
+ title: { display: true, text: 'Monthly savings ($)' },
ticks: { callback: (v) => '$' + (v as number).toLocaleString() },
},
},
diff --git a/frontend/src/index.html b/frontend/src/index.html
index 3e4f178a5..2f796c555 100644
--- a/frontend/src/index.html
+++ b/frontend/src/index.html
@@ -93,13 +93,9 @@ Savings over time
No purchase history yet — the chart will populate once you start executing plans.
-
- Potential Savings by Service
-
-
Potential savings range per service
-
+
No positive potential savings found for current recommendations.
diff --git a/internal/api/handler_dashboard.go b/internal/api/handler_dashboard.go
index 1de157228..763a8d969 100644
--- a/internal/api/handler_dashboard.go
+++ b/internal/api/handler_dashboard.go
@@ -153,6 +153,17 @@ func summarizeRecommendationsWithCoverage(
total += scaled
svc := byService[rec.Service]
svc.PotentialSavings += scaled
+ // CurrentSavings is the committed/realized monthly savings for the
+ // service: the full 100%-coverage potential (rec.Savings) projected
+ // down to the operator-configured coverage %. scaledSavings already
+ // computes exactly that (rec.Savings * min(coverage,100)/100), so we
+ // reuse it rather than re-deriving the coverage lookup. When no
+ // coverage override exists the rec falls through to full savings, so
+ // CurrentSavings == PotentialSavings (nothing committed-away yet); a
+ // configured coverage < 100 pulls CurrentSavings below the potential.
+ // Issue #908: this field was previously never set, so the Home
+ // chart's current-savings underlay always rendered as $0.
+ svc.CurrentSavings += scaled
byService[rec.Service] = svc
}
return total, byService
diff --git a/internal/api/handler_dashboard_test.go b/internal/api/handler_dashboard_test.go
index 43c673ee5..bc20f205f 100644
--- a/internal/api/handler_dashboard_test.go
+++ b/internal/api/handler_dashboard_test.go
@@ -255,6 +255,9 @@ func TestSummarizeRecommendationsWithCoverage(t *testing.T) {
total, byService := summarizeRecommendationsWithCoverage(tc.recs, tc.coverage)
assert.InDelta(t, tc.wantTotal, total, 0.0001)
assert.InDelta(t, tc.wantTotal, byService["rds"].PotentialSavings, 0.0001)
+ // Issue #908: CurrentSavings (committed/realized) is sourced from
+ // the same coverage-scaled amount, so it tracks the scaled total.
+ assert.InDelta(t, tc.wantTotal, byService["rds"].CurrentSavings, 0.0001)
})
}
}
@@ -309,6 +312,46 @@ func TestSummarizeRecommendationsWithCoverage_100PctContract(t *testing.T) {
"nil coverage map must return un-scaled savings (issue #201 contract)")
}
+// TestSummarizeRecommendationsWithCoverage_PopulatesCurrentSavings is the
+// issue #908 regression: by_service[svc].current_savings must be populated
+// (and keyed/scaled identically to potential) so the Home chart's
+// current-savings underlay renders instead of being a flat $0 series.
+//
+// Before the fix, summarizeRecommendationsWithCoverage set only
+// PotentialSavings, leaving CurrentSavings at its float64 zero value for
+// every service regardless of configured coverage.
+func TestSummarizeRecommendationsWithCoverage_PopulatesCurrentSavings(t *testing.T) {
+ acctA := "acct-A"
+ keyEC2 := config.AccountConfigKey(acctA, "aws", "ec2")
+ keyRDS := config.AccountConfigKey(acctA, "aws", "rds")
+
+ recs := []config.RecommendationRecord{
+ {Provider: "aws", Service: "ec2", Savings: 1000.0, CloudAccountID: &acctA},
+ {Provider: "aws", Service: "rds", Savings: 400.0, CloudAccountID: &acctA},
+ }
+
+ // 60% coverage on ec2, 25% on rds.
+ coverage := map[string]float64{keyEC2: 60, keyRDS: 25}
+
+ _, byService := summarizeRecommendationsWithCoverage(recs, coverage)
+
+ // current_savings is non-zero where coverage exists and is scaled the
+ // same way potential is (rec.Savings * coverage/100), keyed per service.
+ assert.InDelta(t, 600.0, byService["ec2"].CurrentSavings, 0.001,
+ "ec2 current_savings = 1000 * 60/100")
+ assert.InDelta(t, 100.0, byService["rds"].CurrentSavings, 0.001,
+ "rds current_savings = 400 * 25/100")
+
+ // And it matches PotentialSavings (both flow from the same scaled amount).
+ assert.InDelta(t, byService["ec2"].PotentialSavings, byService["ec2"].CurrentSavings, 0.001)
+ assert.InDelta(t, byService["rds"].PotentialSavings, byService["rds"].CurrentSavings, 0.001)
+
+ // Sanity: every populated service has a strictly positive current_savings
+ // when coverage is configured (the bug produced 0 here).
+ require.Positive(t, byService["ec2"].CurrentSavings)
+ require.Positive(t, byService["rds"].CurrentSavings)
+}
+
func TestHandler_getUpcomingPurchases(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)